features/plugins_loader_coverage.feature

@@ -1,8 +1,9 @@
 Feature: Plugin Loader Coverage Boost
   Scenarios targeting uncovered lines in the PluginLoader class:
   - Lines 203-209: entry point load failure exception handler
-  - Lines 242, 244-246: validate_protocol fallback to issubclass when instantiation fails
-  - Lines 247-248: validate_protocol issubclass raises TypeError
+  - validate_protocol: issubclass succeeds (class satisfies protocol structurally)
+  - validate_protocol: issubclass raises TypeError with unverifiable protocol
+  - validate_protocol: issubclass returns False (class missing required members)
 
   Background:
     Given the plugin loader module is imported
@@ -18,17 +19,17 @@ Feature: Plugin Loader Coverage Boost
     And the failed entry point should have been logged as a warning
 
   # -----------------------------------------------------------------------
-  # validate_protocol: instantiation fails, issubclass succeeds (lines 242, 244-246)
+  # validate_protocol: issubclass succeeds for class satisfying protocol
   # -----------------------------------------------------------------------
 
-  Scenario: validate_protocol falls back to issubclass when instantiation fails
+  Scenario: validate_protocol returns True when class satisfies protocol via issubclass
     Given I have a class that requires constructor arguments
     And I have a runtime checkable protocol the class satisfies via issubclass
     When I call validate_protocol with the non-instantiable class and protocol
     Then validate_protocol should return True
 
   # -----------------------------------------------------------------------
-  # validate_protocol: instantiation fails, issubclass raises TypeError (lines 247-248)
+  # validate_protocol: issubclass raises TypeError for unverifiable protocol
   # -----------------------------------------------------------------------
 
   Scenario: validate_protocol raises ProtocolMismatchError when issubclass raises TypeError
@@ -38,7 +39,7 @@ Feature: Plugin Loader Coverage Boost
     Then a plugin-loader ProtocolMismatchError should be raised
 
   # -----------------------------------------------------------------------
-  # validate_protocol: instantiation fails, issubclass returns False
+  # validate_protocol: issubclass returns False (class missing required members)
   # -----------------------------------------------------------------------
 
   Scenario: validate_protocol raises ProtocolMismatchError when issubclass returns False

features/steps/plugins_loader_coverage_steps.py

@@ -4,9 +4,9 @@ These steps target specific uncovered lines in
 cleveragents/infrastructure/plugins/loader.py:
 
 - Lines 203-209: except block in load_from_entry_points when ep.load() fails
-- Lines 242, 244-246: validate_protocol fallback to issubclass on
-  instantiation failure
-- Lines 247-248: validate_protocol when issubclass raises TypeError
+- validate_protocol: issubclass succeeds (class satisfies protocol structurally)
+- validate_protocol: issubclass raises TypeError for unverifiable protocol
+- validate_protocol: issubclass returns False (class missing required members)
 """
 
 from typing import Protocol, runtime_checkable
@@ -72,13 +72,13 @@ def step_verify_warning_logged(context):
 
 
 # ---------------------------------------------------------------------------
-# validate_protocol: instantiation fails, issubclass succeeds (lines 242, 244-246)
+# validate_protocol: issubclass succeeds for class satisfying protocol
 # ---------------------------------------------------------------------------
 
 
 @runtime_checkable
 class _SampleProtocol(Protocol):
-    """A simple runtime-checkable Protocol for testing issubclass fallback."""
+    """A simple runtime-checkable Protocol for testing issubclass check."""
 
     def do_work(self) -> str: ...
 
@@ -105,7 +105,7 @@ def step_protocol_satisfied_by_subclass(context):
 
 @when("I call validate_protocol with the non-instantiable class and protocol")
 def step_call_validate_protocol_subclass_fallback(context):
-    """Call validate_protocol; expect it to fall back to issubclass and succeed."""
+    """Call validate_protocol; issubclass succeeds and returns True."""
     context.validate_result = PluginLoader.validate_protocol(
         context.non_instantiable_class,
         context.target_protocol,
@@ -114,18 +114,18 @@ def step_call_validate_protocol_subclass_fallback(context):
 
 @then("validate_protocol should return True")
 def step_verify_validate_true(context):
-    """The fallback issubclass check should have returned True."""
+    """The issubclass check should have returned True."""
     assert context.validate_result is True
 
 
 # ---------------------------------------------------------------------------
-# validate_protocol: instantiation fails, issubclass raises TypeError (lines 247-248)
+# validate_protocol: issubclass raises TypeError for unverifiable protocol
 # ---------------------------------------------------------------------------
 
 
 @given("I have a class that cannot be instantiated without arguments")
 def step_class_cannot_instantiate(context):
-    """Create a class whose __init__ raises when called with no args."""
+    """Create a class whose __init__ requires mandatory arguments."""
 
     class NeedsArgs:
         def __init__(self, required):
@@ -141,10 +141,10 @@ def step_protocol_causes_typeerror(context):
     We need an object that:
     - Has a __name__ attribute (so the error message in validate_protocol works)
     - Causes issubclass() to raise TypeError when used as second arg
-    - Also causes isinstance() to raise TypeError
 
-    A class with a metaclass that raises TypeError on __instancecheck__
-    and __subclasscheck__ achieves this.
+    A class with a metaclass that raises TypeError on __subclasscheck__
+    achieves this. Since this protocol declares no members, the structural
+    fallback cannot verify conformance and must raise ProtocolMismatchError.
     """
 
     class TypeErrorMeta(type):
@@ -186,7 +186,7 @@ def step_verify_protocol_mismatch(context):
 
 
 # ---------------------------------------------------------------------------
-# validate_protocol: instantiation fails, issubclass returns False
+# validate_protocol: issubclass returns False (class missing required members)
 # ---------------------------------------------------------------------------
 
 

src/cleveragents/infrastructure/plugins/loader.py

@@ -241,10 +241,9 @@ class PluginLoader:
     def validate_protocol(klass: type[Any], protocol: type[Any]) -> bool:
         """Check whether *klass* satisfies a ``@runtime_checkable`` Protocol.
 
-        Creates a temporary instance of *klass* (using a no-arg
-        constructor) and checks it against the protocol via
-        ``isinstance``.  If instantiation fails, falls back to a
-        structural check using ``issubclass``.
+        Uses structural type checking via ``issubclass`` to validate the
+        protocol without instantiating the class. This prevents arbitrary
+        code execution from untrusted plugin constructors during validation.
 
         Args:
             klass: The class to validate.
@@ -256,23 +255,84 @@ class PluginLoader:
         Raises:
             ProtocolMismatchError: If *klass* does not satisfy the protocol.
         """
-        # Try instance check first (most reliable for runtime_checkable)
+        # Try structural check first (safe, no instantiation).
+        # This prevents arbitrary code execution from plugin constructors.
+        issubclass_raised_type_error = False
         try:
-            instance = klass()
-            if isinstance(instance, protocol):
+            if issubclass(klass, protocol):
                 return True
-        except Exception:
-            # If instantiation fails, try subclass check
-            try:
-                if issubclass(klass, protocol):
-                    return True
-            except TypeError:
-                pass
+        except TypeError:
+            # issubclass raises TypeError if protocol is not a valid
+            # @runtime_checkable Protocol (e.g. has a custom metaclass that
+            # overrides __subclasscheck__). Fall through to structural check.
+            issubclass_raised_type_error = True
+
+        # Fallback: perform a conservative structural check against the
+        # Protocol definition without instantiating the class. We inspect
+        # the protocol's declared members (callables and annotated names)
+        # and ensure the candidate class exposes them as class-level
+        # attributes or descriptors. This approximates structural
+        # conformance while avoiding constructor execution.
+        required: set[str] = set()
+
+        # Collect names from protocol __dict__ (methods, properties, etc.)
+        prot_dict = getattr(protocol, "__dict__", {})
+        for name, _value in prot_dict.items():
+            if name.startswith("__"):
+                continue
+            # Methods and descriptors will appear in the dict; annotations
+            # are handled below. We treat any non-data-magic name as a
+            # required member.
+            required.add(name)
+
+        # Also include annotated names (those declared with type hints)
+        ann = getattr(protocol, "__annotations__", {}) or {}
+        for name in ann:
+            if name.startswith("__"):
+                continue
+            required.add(name)
+
+        # If issubclass raised TypeError and the protocol declares no
+        # inspectable members, we cannot validate conformance — treat this
+        # as a mismatch rather than silently returning True for an
+        # unverifiable protocol.
+        if issubclass_raised_type_error and not required:
+            msg = (
+                f"Class '{getattr(klass, '__name__', str(klass))}' cannot be "
+                f"validated against protocol "
+                f"'{getattr(protocol, '__name__', str(protocol))}': "
+                f"issubclass raised TypeError and the protocol declares no "
+                f"inspectable members. Ensure the protocol is a valid "
+                f"@runtime_checkable Protocol."
+            )
+            raise ProtocolMismatchError(msg)
+
+        # Validate that the candidate class exposes each required name.
+        missing: list[str] = []
+        for name in sorted(required):
+            # getattr on the class returns descriptors (functions, property,
+            # staticmethod, etc.) without instantiation. hasattr is safe.
+            if not hasattr(klass, name):
+                missing.append(name)
+                continue
+            # If the protocol declared a callable (method), ensure the
+            # attribute on the class is callable or a descriptor.
+            prot_val = prot_dict.get(name)
+            if callable(prot_val):
+                cand = getattr(klass, name)
+                # Properties are descriptors and typically not callable; we
+                # accept them as present. For methods, require callable.
+                if not (callable(cand) or hasattr(cand, "fget")):
+                    missing.append(name)
+
+        if not missing:
+            return True
 
         msg = (
-            f"Class '{klass.__name__}' does not satisfy protocol "
-            f"'{protocol.__name__}'. Ensure the class implements all "
-            f"required methods and attributes."
+            f"Class '{getattr(klass, '__name__', str(klass))}' does not "
+            f"satisfy protocol '{getattr(protocol, '__name__', str(protocol))}'. "
+            f"Missing attributes: {', '.join(missing)}. Ensure the class "
+            f"implements all required methods and descriptors."
         )
         raise ProtocolMismatchError(msg)