diff --git a/pyrit/backend/services/target_service.py b/pyrit/backend/services/target_service.py index 1743e64145..4c3a6e7b8a 100644 --- a/pyrit/backend/services/target_service.py +++ b/pyrit/backend/services/target_service.py @@ -34,10 +34,9 @@ class TargetService: """ Service for managing target instances. - Uses TargetRegistry as the sole source of truth. Class discovery, - construction (incl. param coercion and reference resolution), and endpoint - validation are all owned by the registry and the target classes; this - service only orchestrates the request → registry hand-off. + Uses TargetRegistry as the sole source of truth for class discovery, + parameter coercion, reference resolution, and construction. Endpoint + validation remains owned by the target classes. """ def __init__(self) -> None: @@ -153,18 +152,13 @@ async def create_target_async(self, *, request: CreateTargetRequest) -> TargetIn """ Create a new target instance from API request. - Class discovery is owned by the ``TargetRegistry``. Targets whose build - contract references other registry instances (e.g. ``RoundRobinTarget``'s - ``targets``) are constructed via ``registry.create_instance`` so the - resolver turns registry names into live objects; all other targets carry - their base configuration (``endpoint`` / ``model_name`` / ``api_key``) - through ``**kwargs``, which is not part of the registry's derived - parameter contract, so they are constructed directly from the registry - class. Endpoint trust and identity token minting are owned by the target - classes themselves. This service only enforces the request-level auth - contract: for ``identity`` it confirms the target supports it and omits - the api_key so the target validates its own endpoint and authenticates - itself. + Class discovery, strict parameter validation, scalar coercion, registry + reference resolution, and construction are owned by the + ``TargetRegistry``. Endpoint trust and identity token minting are owned + by the target classes themselves. This service only enforces the + request-level auth contract: for ``identity`` it confirms the target + supports it and omits the api_key so the target validates its own + endpoint and authenticates itself. Args: request: The create target request with type, params, and auth_mode. @@ -192,34 +186,13 @@ async def create_target_async(self, *, request: CreateTargetRequest) -> TargetIn # Omit any api_key so the target validates its own endpoint and authenticates itself. params.pop("api_key", None) - if self._has_reference_params(target_type=request.type): - # e.g. RoundRobinTarget: `targets` is a list of registry names the - # resolver turns into live target objects. - target_obj = self._registry.create_instance(request.type, **params) - else: - target_obj = target_cls(**params) + target_obj = self._registry.create_instance(request.type, **params) self._registry.instances.register(target_obj) target_registry_name = target_obj.get_identifier().unique_name return self._build_instance_from_object(target_registry_name=target_registry_name, target_obj=target_obj) - def _has_reference_params(self, *, target_type: str) -> bool: - """ - Return True if the target type's build contract references other registry - instances (so construction must go through the resolver). - - Args: - target_type (str): The registered target class name. - - Returns: - bool: True if any derived parameter is a registry reference. - """ - metadata = self._registry.get_registered_class_metadata(target_type) - if metadata is None: - return False - return any(param.reference is not None for param in metadata.parameters) - @lru_cache(maxsize=1) def get_target_service() -> TargetService: diff --git a/pyrit/common/__init__.py b/pyrit/common/__init__.py index e7e9b732b9..cd99678728 100644 --- a/pyrit/common/__init__.py +++ b/pyrit/common/__init__.py @@ -21,7 +21,7 @@ reset_default_values, set_default_value, ) -from pyrit.common.brick_contract import enforce_keyword_only_init +from pyrit.common.brick_contract import enforce_keyword_only_init, forward_init_parameters from pyrit.common.default_values import get_non_required_value, get_required_value from pyrit.common.deprecation import print_deprecation_message from pyrit.common.notebook_utils import is_in_ipython_session @@ -43,6 +43,7 @@ "combine_list", "DefaultValueScope", "enforce_keyword_only_init", + "forward_init_parameters", "get_global_default_values", "get_kwarg_param", "get_non_required_value", diff --git a/pyrit/common/brick_contract.py b/pyrit/common/brick_contract.py index d855941f0e..1f3d503e56 100644 --- a/pyrit/common/brick_contract.py +++ b/pyrit/common/brick_contract.py @@ -10,17 +10,57 @@ every subclass must use the keyword-only constructor shape mandated by the style guide: ``def __init__(self, *, ...)``. -This module provides one shared helper, ``enforce_keyword_only_init``, -that bases invoke from their own ``__init_subclass__`` hook. The helper -inspects the subclass's directly-defined ``__init__`` (not inherited) and -classifies it as compliant or non-compliant. Non-compliant subclasses -raise ``TypeError`` at class definition time. +``enforce_keyword_only_init`` validates subclass signatures. +``forward_init_parameters`` explicitly marks constructors that pass their +``**kwargs`` to the next constructor in the MRO, allowing registries to derive +the complete strict build contract without interpreting arbitrary keyword bags. """ from __future__ import annotations import inspect +from collections.abc import Callable from inspect import Parameter +from typing import TypeVar + +_InitMethodT = TypeVar("_InitMethodT", bound=Callable[..., None]) +_FORWARD_INIT_PARAMETERS_ATTRIBUTE = "__pyrit_forward_init_parameters__" + + +def forward_init_parameters(init: _InitMethodT) -> _InitMethodT: + """ + Declare that a constructor forwards ``**kwargs`` to the next MRO constructor. + + The registry uses this explicit declaration to merge parent constructor + parameters into the class's build contract without treating every variadic + keyword bag as parent arguments. + + Args: + init (_InitMethodT): The forwarding constructor. + + Returns: + _InitMethodT: The unchanged constructor with registry metadata attached. + + Raises: + TypeError: If the constructor does not accept ``**kwargs``. + """ + if not any(param.kind is Parameter.VAR_KEYWORD for param in inspect.signature(init).parameters.values()): + raise TypeError("forward_init_parameters requires a constructor that accepts **kwargs.") + setattr(init, _FORWARD_INIT_PARAMETERS_ATTRIBUTE, True) + return init + + +def init_parameters_are_forwarded(init: Callable[..., object]) -> bool: + """ + Return whether a constructor declares that it forwards ``**kwargs``. + + Args: + init (Callable[..., object]): The constructor to inspect. + + Returns: + bool: True when ``forward_init_parameters`` marked the constructor. + """ + return bool(getattr(init, _FORWARD_INIT_PARAMETERS_ATTRIBUTE, False)) def enforce_keyword_only_init(cls: type, *, base_name: str) -> None: diff --git a/pyrit/models/parameter.py b/pyrit/models/parameter.py index d35176172e..14758ac8fc 100644 --- a/pyrit/models/parameter.py +++ b/pyrit/models/parameter.py @@ -191,18 +191,16 @@ def is_string_coercible(self) -> bool: Whether a single string token can be coerced to this parameter's value. True for a non-reference plain scalar (``str`` / ``int`` / ``float`` / - ``bool``) or ``Literal[...]`` parameter — exactly the forms a text field or - CLI token can supply. References and structured types (lists, enums, - arbitrary objects) are False and are surfaced/handled elsewhere. + ``bool``), ``Literal[...]``, or ``Enum`` parameter — exactly the forms a + text field or CLI token can supply. References and structured types (lists + and arbitrary objects) are False and are surfaced/handled elsewhere. Returns: bool: True when a string can be coerced to this parameter's value. """ - if self.reference is not None: + if self.reference is not None or self.opaque: return False - if self.param_type in _SUPPORTED_SCALAR_TYPES: - return True - return get_origin(self.param_type) is Literal + return _is_scalar_param_type(_unwrap_optional(self.param_type)) def is_reference_to(self, component_type: ComponentType) -> bool: """ @@ -246,7 +244,7 @@ def coerce_value(self, raw_value: Any) -> Any: """ if self.reference is not None or self.opaque: return raw_value - param_type = self.param_type + param_type = _unwrap_optional(self.param_type) if param_type is None: return copy.deepcopy(raw_value) if get_origin(param_type) is list: diff --git a/pyrit/prompt_target/openai/openai_chat_target.py b/pyrit/prompt_target/openai/openai_chat_target.py index 4da1ed77c8..213633b13c 100644 --- a/pyrit/prompt_target/openai/openai_chat_target.py +++ b/pyrit/prompt_target/openai/openai_chat_target.py @@ -5,6 +5,7 @@ from collections.abc import MutableSequence from typing import Any +from pyrit.common import forward_init_parameters from pyrit.exceptions import ( EmptyResponseException, pyrit_target_retry, @@ -89,6 +90,7 @@ class OpenAIChatTarget(OpenAITarget): ) ) + @forward_init_parameters def __init__( self, *, diff --git a/pyrit/prompt_target/openai/openai_completion_target.py b/pyrit/prompt_target/openai/openai_completion_target.py index 998c700196..af34dcc270 100644 --- a/pyrit/prompt_target/openai/openai_completion_target.py +++ b/pyrit/prompt_target/openai/openai_completion_target.py @@ -4,6 +4,7 @@ import logging from typing import Any +from pyrit.common import forward_init_parameters from pyrit.exceptions.exception_classes import ( pyrit_target_retry, ) @@ -21,6 +22,7 @@ class OpenAICompletionTarget(OpenAITarget): _DEFAULT_CONFIGURATION: TargetConfiguration = TargetConfiguration(capabilities=TargetCapabilities()) + @forward_init_parameters def __init__( self, *, diff --git a/pyrit/prompt_target/openai/openai_image_target.py b/pyrit/prompt_target/openai/openai_image_target.py index e9ffccef3b..9c96041313 100644 --- a/pyrit/prompt_target/openai/openai_image_target.py +++ b/pyrit/prompt_target/openai/openai_image_target.py @@ -4,6 +4,7 @@ import logging from typing import Any, Literal +from pyrit.common import forward_init_parameters from pyrit.exceptions import ( EmptyResponseException, pyrit_target_retry, @@ -41,6 +42,7 @@ class OpenAIImageTarget(OpenAITarget): ) ) + @forward_init_parameters def __init__( self, *, diff --git a/pyrit/prompt_target/openai/openai_realtime_target.py b/pyrit/prompt_target/openai/openai_realtime_target.py index 38da2d88e7..3894ddc623 100644 --- a/pyrit/prompt_target/openai/openai_realtime_target.py +++ b/pyrit/prompt_target/openai/openai_realtime_target.py @@ -10,6 +10,7 @@ from openai import AsyncOpenAI +from pyrit.common import forward_init_parameters from pyrit.exceptions import ( pyrit_target_retry, ) @@ -79,6 +80,7 @@ class RealtimeTarget(OpenAITarget): #: of truth for both atomic (send_text/send_audio) and streaming session paths. SAMPLE_RATE_HZ: ClassVar[int] = 24000 + @forward_init_parameters def __init__( self, *, diff --git a/pyrit/prompt_target/openai/openai_response_target.py b/pyrit/prompt_target/openai/openai_response_target.py index 563dfaeb2d..59913f853f 100644 --- a/pyrit/prompt_target/openai/openai_response_target.py +++ b/pyrit/prompt_target/openai/openai_response_target.py @@ -14,6 +14,7 @@ from openai.types.shared import ReasoningEffort +from pyrit.common import forward_init_parameters from pyrit.exceptions import ( EmptyResponseException, PyritException, @@ -95,6 +96,7 @@ class OpenAIResponseTarget(OpenAITarget): ) ) + @forward_init_parameters def __init__( self, *, diff --git a/pyrit/prompt_target/openai/openai_tts_target.py b/pyrit/prompt_target/openai/openai_tts_target.py index 03602c31a4..63591c97ff 100644 --- a/pyrit/prompt_target/openai/openai_tts_target.py +++ b/pyrit/prompt_target/openai/openai_tts_target.py @@ -4,6 +4,7 @@ import logging from typing import Any, Literal +from pyrit.common import forward_init_parameters from pyrit.exceptions import ( pyrit_target_retry, ) @@ -30,6 +31,7 @@ class OpenAITTSTarget(OpenAITarget): ) ) + @forward_init_parameters def __init__( self, *, diff --git a/pyrit/prompt_target/openai/openai_video_target.py b/pyrit/prompt_target/openai/openai_video_target.py index 0d83d7ed74..eedfda4258 100644 --- a/pyrit/prompt_target/openai/openai_video_target.py +++ b/pyrit/prompt_target/openai/openai_video_target.py @@ -8,6 +8,7 @@ from openai.types import VideoSeconds, VideoSize +from pyrit.common import forward_init_parameters from pyrit.exceptions import ( pyrit_target_retry, ) @@ -56,6 +57,7 @@ class OpenAIVideoTarget(OpenAITarget): ) ) + @forward_init_parameters def __init__( self, *, diff --git a/pyrit/registry/resolution.py b/pyrit/registry/resolution.py index cf4c4de6b3..98534e3ba8 100644 --- a/pyrit/registry/resolution.py +++ b/pyrit/registry/resolution.py @@ -9,12 +9,13 @@ from a class ``__init__`` or declared explicitly by a component. It has three responsibilities: -- **Derive** (``derive_parameters``): read the constructor signature, enriched - by the identifier's ``Param.*`` build markers, into a ``list[Parameter]``. A - parameter the identifier promotes as a reference to another registry (an - included field typed as a child identifier, e.g. ``TargetIdentifier``) becomes - a registry **reference**; every other parameter becomes a plain value parameter - whose ``param_type`` is the annotation with ``Optional[X]`` reduced to ``X``. +- **Derive** (``derive_parameters``): read the constructor signature, plus + explicitly forwarded parent signatures in MRO order, and enrich them with the + identifier's ``Param.*`` build markers into a ``list[Parameter]``. A parameter + the identifier promotes as a reference to another registry (an included field + typed as a child identifier, e.g. ``TargetIdentifier``) becomes a registry + **reference**; every other parameter becomes a plain value parameter whose + ``param_type`` is the annotation with ``Optional[X]`` reduced to ``X``. - **Resolve from a constructor** (``resolve_constructor_args``): derive the contract for a class and turn a flat dict of raw arguments into constructor-ready keyword arguments — coercing simple string values via @@ -42,6 +43,7 @@ from typing import TYPE_CHECKING, Any, Protocol, TypeAlias, Union, get_args, get_origin from pyrit.common.apply_defaults import REQUIRED_VALUE, _RequiredValueSentinel +from pyrit.common.brick_contract import init_parameters_are_forwarded from pyrit.models.parameter import ComponentType, Parameter, RegistryReference # Re-exported so ``from pyrit.registry.resolution import display_choices`` keeps working; @@ -121,70 +123,127 @@ def _default_for(param: inspect.Parameter) -> Any: return param.default -def derive_parameters(*, cls: type, identifier_type: type[ComponentIdentifier] | None = None) -> list[Parameter]: +def _constructor_sources(cls: type) -> list[tuple[type, inspect.Signature]]: """ - Derive the declarative ``Parameter`` list for ``cls`` from its constructor. + Return the constructor signatures that form ``cls``'s build contract. - Performs the single ``inspect.signature`` call of the build pipeline and maps - each settable constructor parameter to a ``Parameter``: parameters the - identifier promotes as references carry a ``RegistryReference``; plain - parameters carry an ``Optional``-unwrapped ``param_type``. Parameter order - follows the constructor signature. + The effective constructor is always first. When it explicitly declares + ``**kwargs`` forwarding with ``forward_init_parameters``, the next constructor + defined along the MRO is included. The same rule is applied recursively. Args: - cls (type): The component class whose ``__init__`` drives derivation. - identifier_type (type[ComponentIdentifier] | None): The domain identifier - whose ``Param.*`` markers declare which parameters are registry - references. When None, no parameter is treated as a reference. + cls (type): The class whose constructor chain is inspected. Returns: - list[Parameter]: One ``Parameter`` per settable constructor parameter. + list[tuple[type, inspect.Signature]]: Constructor owners and signatures + in child-to-parent order. Raises: - ValueError: If the constructor signature cannot be inspected. + ValueError: If a constructor signature cannot be inspected. """ - try: - sig = inspect.signature(cls.__init__) - except (ValueError, TypeError) as e: - raise ValueError(f"Failed to inspect __init__ signature for '{cls.__name__}': {e}") from e + owners = [owner for owner in cls.__mro__ if "__init__" in owner.__dict__] + sources: list[tuple[type, inspect.Signature]] = [] + for index, owner in enumerate(owners): + init = owner.__dict__["__init__"] + try: + signature = inspect.signature(init) + except (ValueError, TypeError) as exc: + raise ValueError(f"Failed to inspect __init__ signature for '{cls.__name__}': {exc}") from exc + sources.append((owner, signature)) - reference_overrides = identifier_type.get_reference_component_types() if identifier_type is not None else {} - descriptions = _parse_arg_descriptions(cls) + if not init_parameters_are_forwarded(init) or index + 1 == len(owners): + break + return sources + + +def _parameters_from_signature( + *, + owner: type, + signature: inspect.Signature, + reference_overrides: dict[str, ComponentType], +) -> list[Parameter]: + """ + Build parameters declared by one constructor signature. + Args: + owner (type): The class that defines the constructor. + signature (inspect.Signature): The constructor signature. + reference_overrides (dict[str, ComponentType]): Identifier-declared + registry references keyed by constructor parameter name. + + Returns: + list[Parameter]: Parameters declared by the constructor. + """ + descriptions = _parse_arg_descriptions(owner) parameters: list[Parameter] = [] - for name, param in sig.parameters.items(): - if name in _SKIPPED_PARAM_NAMES: - continue - if param.kind in ( + for name, param in signature.parameters.items(): + if name in _SKIPPED_PARAM_NAMES or param.kind in ( inspect.Parameter.VAR_POSITIONAL, inspect.Parameter.VAR_KEYWORD, ): continue - annotation = param.annotation component_type = reference_overrides.get(name) - description = descriptions.get(name, "") - default = _default_for(param) - if component_type is not None: parameters.append( Parameter( name=name, - description=description, - default=default, - reference=RegistryReference(component_type=component_type, annotation=annotation), + description=descriptions.get(name, ""), + default=_default_for(param), + reference=RegistryReference(component_type=component_type, annotation=param.annotation), ) ) - else: - param_type = None if annotation is inspect.Parameter.empty else _unwrap_optional(annotation) - parameters.append( - Parameter( - name=name, - description=description, - default=default, - param_type=param_type, - ) + continue + + param_type = None if param.annotation is inspect.Parameter.empty else _unwrap_optional(param.annotation) + parameters.append( + Parameter( + name=name, + description=descriptions.get(name, ""), + default=_default_for(param), + param_type=param_type, ) + ) + return parameters + + +def derive_parameters(*, cls: type, identifier_type: type[ComponentIdentifier] | None = None) -> list[Parameter]: + """ + Derive the declarative ``Parameter`` list for ``cls`` from its constructor. + + Maps each settable constructor parameter to a ``Parameter``: parameters the + identifier promotes as references carry a ``RegistryReference``; plain + parameters carry an ``Optional``-unwrapped ``param_type``. When a constructor + explicitly declares that its ``**kwargs`` are forwarded, the next constructor + in MRO order is merged. Child declarations take precedence over same-named + base declarations. + + Args: + cls (type): The component class whose ``__init__`` drives derivation. + identifier_type (type[ComponentIdentifier] | None): The domain identifier + whose ``Param.*`` markers declare which parameters are registry + references. When None, no parameter is treated as a reference. + + Returns: + list[Parameter]: One ``Parameter`` per settable constructor parameter, + ordered from the effective constructor through forwarded bases. + + Raises: + ValueError: If the constructor signature cannot be inspected. + """ + reference_overrides = identifier_type.get_reference_component_types() if identifier_type is not None else {} + parameters: list[Parameter] = [] + seen: set[str] = set() + for owner, signature in _constructor_sources(cls): + for parameter in _parameters_from_signature( + owner=owner, + signature=signature, + reference_overrides=reference_overrides, + ): + if parameter.name in seen: + continue + parameters.append(parameter) + seen.add(parameter.name) return parameters @@ -379,11 +438,10 @@ def resolve_constructor_args( """ Resolve a flat argument dict into constructor-ready keyword arguments. - Derives the ``Parameter`` contract for ``cls`` (the single - ``inspect.signature`` call) and applies it to ``raw_args``. For each raw - argument: validate it is a declared parameter; resolve registry-reference - parameters by name; coerce simple string values via - ``Parameter.coerce_value``; pass everything else through unchanged. + Derives the ``Parameter`` contract for ``cls`` and applies it to + ``raw_args``. For each raw argument: validate it is a declared parameter; + resolve registry-reference parameters by name; coerce simple string values + via ``Parameter.coerce_value``; pass everything else through unchanged. Args: cls (type): The class being built. diff --git a/tests/unit/backend/test_target_service.py b/tests/unit/backend/test_target_service.py index 8a70b7c9bc..9984b33e9e 100644 --- a/tests/unit/backend/test_target_service.py +++ b/tests/unit/backend/test_target_service.py @@ -253,6 +253,62 @@ async def test_catalog_includes_declarative_auth_facts(self) -> None: assert "api_key" in openai_entry.supported_auth_modes assert "identity" in openai_entry.supported_auth_modes + @pytest.mark.parametrize( + ("target_type", "parameter_name", "type_name", "required", "choices"), + [ + ( + "GandalfTarget", + "level", + "GandalfLevel", + True, + [ + "baseline", + "do-not-tell", + "do-not-tell-and-block", + "gpt-is-password-encoded", + "word-blacklist", + "gpt-blacklist", + "gandalf", + "gandalf-the-white", + "adventure-1", + "adventure-2", + ], + ), + ( + "AzureBlobStorageTarget", + "blob_content_type", + "SupportedContentType", + False, + ["text/plain", "text/html"], + ), + ( + "PlaywrightCopilotTarget", + "copilot_type", + "CopilotType", + False, + ["consumer", "m365"], + ), + ], + ) + async def test_catalog_includes_enum_parameters( + self, + target_type: str, + parameter_name: str, + type_name: str, + required: bool, + choices: list[str], + ) -> None: + """Enum parameters are exposed with their required state and allowed values.""" + service = TargetService() + + result = await service.list_target_catalog_async() + + entry = next(item for item in result.items if item.target_type == target_type) + parameter = next(param for param in entry.parameters if param.name == parameter_name) + assert parameter.required is required + assert parameter.type_name == type_name + assert parameter.choices == choices + class TestCreateTarget: """Tests for TargetService.create_target method.""" @@ -283,6 +339,55 @@ async def test_create_target_success(self, sqlite_instance) -> None: assert result.target_registry_name is not None assert result.identifier.class_name == "TextTarget" + async def test_create_target_delegates_construction_to_registry(self, sqlite_instance) -> None: + """Every target construction path is owned by the registry.""" + service = TargetService() + with patch.object(service._registry, "create_instance", wraps=service._registry.create_instance) as create: + await service.create_target_async(request=CreateTargetRequest(type="TextTarget", params={})) + + create.assert_called_once() + + async def test_create_gandalf_target_coerces_level_string(self, sqlite_instance) -> None: + """A Gandalf level from the JSON request is coerced to its enum before construction.""" + service = TargetService() + request = CreateTargetRequest( + type="GandalfTarget", + params={"level": "baseline"}, + ) + + result = await service.create_target_async(request=request) + + assert result.identifier.class_name == "GandalfTarget" + assert result.target_specific_params == {"level": "baseline"} + + async def test_create_gandalf_target_rejects_invalid_level(self, sqlite_instance) -> None: + """An invalid Gandalf level raises a parameter error before target construction.""" + service = TargetService() + request = CreateTargetRequest( + type="GandalfTarget", + params={"level": "unknown"}, + ) + + with pytest.raises(ValueError, match="Parameter 'level'.*expected one of"): + await service.create_target_async(request=request) + + async def test_create_azure_blob_target_coerces_content_type_string(self, sqlite_instance) -> None: + """An explicit blob content type from JSON is coerced before target construction.""" + service = TargetService() + request = CreateTargetRequest( + type="AzureBlobStorageTarget", + params={ + "container_url": "https://test.blob.core.windows.net/test", + "sas_token": "valid_sas_token", + "blob_content_type": "text/html", + }, + ) + + result = await service.create_target_async(request=request) + + target = service.get_target_object(target_registry_name=result.target_registry_name) + assert target._blob_content_type == "text/html" + async def test_create_target_registers_in_registry(self, sqlite_instance) -> None: """Test that create_target registers object in registry.""" service = TargetService() diff --git a/tests/unit/common/test_brick_contract.py b/tests/unit/common/test_brick_contract.py index a782c6c7ed..cd674ace46 100644 --- a/tests/unit/common/test_brick_contract.py +++ b/tests/unit/common/test_brick_contract.py @@ -3,7 +3,11 @@ import pytest -from pyrit.common.brick_contract import enforce_keyword_only_init +from pyrit.common.brick_contract import ( + enforce_keyword_only_init, + forward_init_parameters, + init_parameters_are_forwarded, +) class _FakeBase: @@ -14,6 +18,22 @@ def __init_subclass__(cls, **kwargs: object) -> None: enforce_keyword_only_init(cls, base_name="_FakeBase") +def test_forward_init_parameters_marks_variadic_constructor() -> None: + @forward_init_parameters + def _init(self: object, **kwargs: object) -> None: + pass + + assert init_parameters_are_forwarded(_init) + + +def test_forward_init_parameters_rejects_non_variadic_constructor() -> None: + with pytest.raises(TypeError, match=r"requires a constructor that accepts \*\*kwargs"): + + @forward_init_parameters + def _init(self: object, *, value: str) -> None: + pass + + def test_compliant_keyword_only_init_passes() -> None: class Compliant(_FakeBase): def __init__(self, *, foo: str, bar: int = 0) -> None: diff --git a/tests/unit/models/test_parameter.py b/tests/unit/models/test_parameter.py index edb7afd662..d65362c109 100644 --- a/tests/unit/models/test_parameter.py +++ b/tests/unit/models/test_parameter.py @@ -160,12 +160,15 @@ def test_unconstrained_returns_none(self, annotation: object) -> None: class TestIsStringCoercible: """``Parameter.is_string_coercible`` reflects whether a string token can supply the value.""" - @pytest.mark.parametrize("param_type", [str, int, float, bool, Literal["a", "b"]]) + @pytest.mark.parametrize( + "param_type", + [str, int, float, bool, Literal["a", "b"], _Speed, int | None, _Speed | None], + ) def test_coercible_value_types(self, param_type: object) -> None: p = Parameter(name="x", description="d", param_type=param_type) assert p.is_string_coercible is True - @pytest.mark.parametrize("param_type", [None, list[str], _Speed, _Unsupported]) + @pytest.mark.parametrize("param_type", [None, list[str], _Unsupported]) def test_non_coercible_value_types(self, param_type: object) -> None: p = Parameter(name="x", description="d", param_type=param_type) assert p.is_string_coercible is False @@ -178,6 +181,10 @@ def test_reference_is_never_coercible(self) -> None: ) assert p.is_string_coercible is False + def test_opaque_is_never_coercible(self) -> None: + p = Parameter(name="value", description="d", param_type=str, opaque=True) + assert p.is_string_coercible is False + class TestIsReferenceTo: """``Parameter.is_reference_to`` is the single predicate for "points at this component family".""" @@ -259,6 +266,10 @@ def test_enum_by_member(self) -> None: p = Parameter(name="speed", description="d", param_type=_Speed) assert p.coerce_value(_Speed.SLOW) is _Speed.SLOW + def test_optional_enum_by_value(self) -> None: + p = Parameter(name="speed", description="d", param_type=_Speed | None) + assert p.coerce_value("slow") is _Speed.SLOW + def test_enum_invalid_raises(self) -> None: p = Parameter(name="speed", description="d", param_type=_Speed) with pytest.raises(ValueError, match="one of"): diff --git a/tests/unit/registry/test_resolution.py b/tests/unit/registry/test_resolution.py index 6857a04fd0..fab942843e 100644 --- a/tests/unit/registry/test_resolution.py +++ b/tests/unit/registry/test_resolution.py @@ -5,11 +5,12 @@ Tests for the shared registry constructor-argument resolution primitive. """ +from enum import Enum from typing import Literal import pytest -from pyrit.common import REQUIRED_VALUE +from pyrit.common import REQUIRED_VALUE, forward_init_parameters from pyrit.common.apply_defaults import _RequiredValueSentinel from pyrit.models import Message, MessagePiece from pyrit.models.identifiers import ConverterIdentifier, TargetIdentifier @@ -58,6 +59,18 @@ def __init__( self.mode = mode +class _Speed(Enum): + FAST = "fast" + SLOW = "slow" + + +class _EnumOnly: + """Helper whose constructor takes an enum parameter.""" + + def __init__(self, *, speed: _Speed) -> None: + self.speed = speed + + class _Plain: def __init__( self, *, count: int, ratio: float = 0.5, mode: Literal["a", "b"] = "a", note: str | None = None @@ -86,6 +99,31 @@ def __init__(self, *args: object, name: str = "n", **kwargs: object) -> None: self.name = name +class _ForwardedParent: + def __init__(self, *, count: int = 1, speed: _Speed = _Speed.FAST) -> None: + self.count = count + self.speed = speed + + +class _ForwardingChild(_ForwardedParent): + @forward_init_parameters + def __init__(self, *, label: str = "child", **kwargs: object) -> None: + super().__init__(**kwargs) + self.label = label + + +class _OpaqueParent: + def __init__(self, *, count: int = 1) -> None: + self.count = count + + +class _OpenBagChild(_OpaqueParent): + def __init__(self, *, label: str = "child", **options: object) -> None: + super().__init__() + self.label = label + self.options = options + + class _StrTargetArg: """A constructor arg named like the identifier reference but annotated as a plain type.""" @@ -153,6 +191,18 @@ def test_literal_invalid_raises(self) -> None: with pytest.raises(ValueError, match="mode"): _resolve(_SimpleOnly, {"mode": "z"}) + def test_enum_string_coerces_to_member(self) -> None: + resolved = _resolve(_EnumOnly, {"speed": "fast"}) + assert resolved == {"speed": _Speed.FAST} + + def test_forwarded_parent_params_are_coerced(self) -> None: + resolved = _resolve(_ForwardingChild, {"label": "configured", "count": "3", "speed": "slow"}) + assert resolved == {"label": "configured", "count": 3, "speed": _Speed.SLOW} + + def test_open_bag_does_not_disable_unknown_param_rejection(self) -> None: + with pytest.raises(ValueError, match="Unknown parameter 'count'"): + _resolve(_OpenBagChild, {"count": "3"}) + def test_unknown_param_raises(self) -> None: with pytest.raises(ValueError, match="Unknown parameter 'nope'"): _resolve(_SimpleOnly, {"nope": "1"}) @@ -240,6 +290,14 @@ def test_var_args_skipped(self) -> None: names = [p.name for p in derive_parameters(cls=_VarArgs)] assert names == ["name"] + def test_forwarded_parent_parameters_follow_child_in_mro_order(self) -> None: + names = [p.name for p in derive_parameters(cls=_ForwardingChild)] + assert names == ["label", "count", "speed"] + + def test_unexposed_parent_parameters_are_not_inferred_from_open_bag(self) -> None: + names = [p.name for p in derive_parameters(cls=_OpenBagChild)] + assert names == ["label"] + def test_identifier_marker_overrides_plain_annotation(self) -> None: # The identifier marks ``converter_target`` as a TARGET reference, so even a # plainly-annotated arg of that name becomes a reference (the marker wins). diff --git a/tests/unit/registry/test_target_registry.py b/tests/unit/registry/test_target_registry.py index e77e0d53bb..da753d5076 100644 --- a/tests/unit/registry/test_target_registry.py +++ b/tests/unit/registry/test_target_registry.py @@ -11,6 +11,8 @@ from pyrit.models import ComponentIdentifier, Message, MessagePiece from pyrit.models.parameter import ComponentType from pyrit.prompt_target import ( + CopilotType, + PlaywrightCopilotTarget, PromptTarget, RoundRobinTarget, TargetCapabilities, @@ -55,6 +57,13 @@ def _validate_request(self, *, normalized_conversation: list[Message]) -> None: pass +class _PageStub: + """Minimal page object for constructing a Playwright Copilot target.""" + + def __init__(self, *, url: str) -> None: + self.url = url + + @pytest.fixture def registry(): """Provide a fresh ``TargetRegistry`` singleton, reset around each test.""" @@ -272,6 +281,34 @@ def test_build_round_robin_scalar_for_list_reference_raises(self, registry: Targ with pytest.raises(ValueError, match="expected a list"): registry.create_instance("RoundRobinTarget", targets="t1") + def test_build_playwright_copilot_coerces_type_string(self, registry: TargetRegistry) -> None: + target = registry.create_instance( + "PlaywrightCopilotTarget", + page=_PageStub(url="https://m365.microsoft.com/copilot"), + copilot_type="m365", + ) + + assert isinstance(target, PlaywrightCopilotTarget) + assert target._type is CopilotType.M365 + assert target.get_identifier().params["copilot_type"] == "m365" + + def test_build_openai_chat_accepts_forwarded_base_parameters(self, registry: TargetRegistry) -> None: + target = registry.create_instance( + "OpenAIChatTarget", + endpoint="https://test.openai.azure.com/", + model_name="gpt-4o", + api_key="test-key", + max_requests_per_minute="12", + ) + + assert target._endpoint == "https://test.openai.azure.com/" + assert target._model_name == "gpt-4o" + assert target._max_requests_per_minute == 12 + + def test_build_openai_chat_rejects_unknown_parameter(self, registry: TargetRegistry) -> None: + with pytest.raises(ValueError, match="Unknown parameter 'unknown'"): + registry.create_instance("OpenAIChatTarget", unknown="value") + def test_unknown_type_raises(self, registry: TargetRegistry): with pytest.raises(KeyError, match="not found"): registry.create_instance("NotARealTarget") @@ -310,6 +347,13 @@ def test_metadata_supported_auth_modes_sourced_from_class_attributes(self, regis assert "supported_auth_modes" in meta.class_attributes assert meta.class_attributes["supported_auth_modes"] == ("api_key", "identity") + def test_openai_metadata_includes_forwarded_base_parameters(self, registry: TargetRegistry) -> None: + params = {param.name: param for param in self._metadata_for(registry, "OpenAIChatTarget").parameters} + + assert params["endpoint"].param_type is str + assert params["model_name"].param_type is str + assert "api_key" in params + class TestRegistrationGate: """The identifier blueprint must line up with a resolvable contract for every target."""