diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 705378824..57b8eb0e1 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -4,7 +4,7 @@ PyRIT (Python Risk Identification Tool for generative AI) is an open-source fram ## Architecture -PyRIT uses a modular "Lego brick" design. The main extensibility points are: +PyRIT uses a modular pluggable-brick design. The main extensibility points are: - **Prompt Converters** (`pyrit/prompt_converter/`) — Transform prompts (70+ implementations). Base: `PromptConverter`. - **Scorers** (`pyrit/score/`) — Evaluate responses. Base: `Scorer`. diff --git a/.github/instructions/attacks.instructions.md b/.github/instructions/attacks.instructions.md new file mode 100644 index 000000000..2d5c4d7c9 --- /dev/null +++ b/.github/instructions/attacks.instructions.md @@ -0,0 +1,58 @@ +--- +applyTo: "pyrit/executor/attack/**" +--- + +# PyRIT AttackStrategy Development Guidelines + +`AttackStrategy` subclasses (single-turn attacks like `PromptSendingAttack`, multi-turn attacks like `RedTeamingAttack`, etc.) are pluggable bricks orchestrated by `AttackExecutor` and the `Scenario` framework. Style rules from `style-guide.instructions.md` (async `_async` suffix, keyword-only args, type hints, enums-over-Literals) still apply and are not repeated here. + +## Constructor contract + +`AttackStrategy` subclasses MUST follow the keyword-only constructor shape: + +```python +class MyAttack(AttackStrategy[MyContext, MyResult]): + def __init__( + self, + *, + objective_target: PromptTarget, + custom_param: str = "", + **kwargs: Any, + ) -> None: + super().__init__( + objective_target=objective_target, + context_type=MyContext, + **kwargs, + ) +``` + +Requirements: + +- All parameters after ``self`` are keyword-only (insert ``*`` immediately + after ``self``). This is **enforced at class-definition time** by + `AttackStrategy.__init_subclass__` calling `enforce_keyword_only_init` + (see `pyrit/common/brick_contract.py`). Non-conforming subclasses + raise `TypeError` at import time. +- ``super().__init__(...)`` must be invoked with at minimum + ``objective_target`` and ``context_type``. +- Existing subclasses that cannot adopt the contract immediately may set + the class attribute ``_brick_legacy_init = True`` to opt into a + one-release grace period that downgrades the error to a + ``DeprecationWarning(removed_in="0.16.0")``. The opt-out is removed in + 0.16.0; classes that still violate the contract at that point will hard + fail. +- ``AttackTechniqueFactory`` already rejects ``**kwargs`` in attack + ``__init__`` at factory-registration time + (`pyrit/scenario/core/attack_technique_factory.py`); the new + ``__init_subclass__`` check is complementary — the factory check catches + scenarios-side wiring mistakes, the subclass check catches the + ``__init__`` shape at class-definition time. + +## Common pitfalls + +- Forgetting ``*`` after ``self`` — the new check will surface this at + import time with the exact list of positional parameters that need to be + made keyword-only. +- Calling ``super().__init__`` with positional arguments — the base + ``AttackStrategy.__init__`` is already keyword-only, so positional calls + raise ``TypeError`` at runtime. Always forward via kwargs. diff --git a/.github/instructions/converters.instructions.md b/.github/instructions/converters.instructions.md index 695824a9c..f409395e0 100644 --- a/.github/instructions/converters.instructions.md +++ b/.github/instructions/converters.instructions.md @@ -80,6 +80,42 @@ class MyConverter(PromptConverter): ... ``` +### Keyword-only ``__init__`` is enforced + +Every ``PromptConverter`` subclass MUST make all ``__init__`` parameters +keyword-only (i.e., place ``*`` as the first parameter after ``self``). +``PromptConverter.__init_subclass__`` validates this at class-definition +time via ``enforce_keyword_only_init`` and raises ``TypeError`` on +violations. + +The check is satisfied by either of: + +```python +def __init__(self, *, foo: str, bar: int = 0) -> None: ... + +def __init__(self, *args: str, foo: str = "") -> None: ... # *args after self +``` + +It rejects: + +```python +def __init__(self, foo: str, bar: int = 0) -> None: ... # missing * +``` + +### Temporary opt-out: ``_brick_legacy_init`` + +A handful of legacy converters whose positional ``__init__`` is part of the +public API are grandfathered with ``_brick_legacy_init = True``. They +emit a ``DeprecationWarning`` at import time and the opt-out is scheduled +for removal in **0.16.0**. Do not set this flag on new converters; new +converters MUST follow the keyword-only contract. + +Currently grandfathered (slated for cleanup in 0.16.0): +``AddImageVideoConverter``, ``AnsiAttackConverter``, ``AsciiArtConverter``, +``AskToDecodeConverter``, ``DiacriticConverter``, ``InsertPunctuationConverter``, +``PDFConverter``, ``QRCodeConverter``, ``RandomCapitalLettersConverter``, +``SearchReplaceConverter``, ``SmugglerConverter`` (and its three subclasses). + ## Exports and External Updates - New converters MUST be added to `pyrit/prompt_converter/__init__.py` — both the import and the `__all__` list. diff --git a/.github/instructions/datasets.instructions.md b/.github/instructions/datasets.instructions.md index 1a95d3d9a..4e2bbf800 100644 --- a/.github/instructions/datasets.instructions.md +++ b/.github/instructions/datasets.instructions.md @@ -7,6 +7,13 @@ applyTo: "pyrit/datasets/seed_datasets/**" These rules apply when adding or modifying loaders under `pyrit/datasets/seed_datasets/`. Style rules from `style-guide.instructions.md` (async `_async` suffix, keyword-only args, type hints, enums-over-Literals) still apply and are not repeated here. +The keyword-only `__init__` rule is **enforced at class-definition time** by +`SeedDatasetProvider.__init_subclass__` calling `enforce_keyword_only_init` (see +`pyrit/common/brick_contract.py`). Loaders with positional `__init__` params raise +`TypeError` at import time; existing offenders may set `_brick_legacy_init = True` +to opt into a one-release grace period that downgrades the error to a +`DeprecationWarning(removed_in="0.16.0")`. + ## Use SeedObjective for behavior/goal rows; SeedPrompt for literal messages This is the most consequential modelling decision and must be made before writing the loader: diff --git a/.github/instructions/scenarios.instructions.md b/.github/instructions/scenarios.instructions.md index 69e28489d..2f5efefb8 100644 --- a/.github/instructions/scenarios.instructions.md +++ b/.github/instructions/scenarios.instructions.md @@ -71,7 +71,13 @@ def __init__( Requirements: - `@apply_defaults` decorator on `__init__` -- All parameters keyword-only via `*` +- All parameters keyword-only via `*` — **enforced at class-definition time** by + `Scenario.__init_subclass__` calling `enforce_keyword_only_init` (see + `pyrit/common/brick_contract.py`). Violators raise `TypeError` at + import time. Existing classes that cannot adopt the contract immediately + may opt into a one-release grace period via the class attribute + `_brick_legacy_init = True`, which downgrades the error to a + `DeprecationWarning(removed_in="0.16.0")`. The opt-out is removed in 0.16.0. - **All constructor parameters must be optional** (default to `None`) so the registry can instantiate the scenario with no arguments for metadata introspection. Defer required-input validation to `initialize_async()` or `_get_atomic_attacks_async()`. `ScenarioRegistry._build_metadata` raises `TypeError` if `scenario_class()` cannot be called with no arguments. - `super().__init__()` called with `version`, `strategy_class`, `default_strategy`, `default_dataset_config`, `objective_scorer` - complex objects like `adversarial_chat` or `objective_scorer` should be passed into the constructor. diff --git a/.github/instructions/scorers.instructions.md b/.github/instructions/scorers.instructions.md new file mode 100644 index 000000000..b4200704e --- /dev/null +++ b/.github/instructions/scorers.instructions.md @@ -0,0 +1,60 @@ +--- +applyTo: "pyrit/score/**" +--- + +# PyRIT Scorer Development Guidelines + +Scorers evaluate model responses against an objective and live under `pyrit/score/`. Style rules from `style-guide.instructions.md` (async `_async` suffix, keyword-only args, type hints, enums-over-Literals) still apply and are not repeated here. + +## Constructor contract + +`Scorer` subclasses MUST use the keyword-only constructor shape: + +```python +class MyScorer(Scorer): + def __init__( + self, + *, + chat_target: PromptTarget | None = None, + threshold: float = 0.5, + validator: ScorerPromptValidator | None = None, + ) -> None: + super().__init__( + validator=validator or self._DEFAULT_VALIDATOR, + chat_target=chat_target, + ) +``` + +Requirements: + +- All parameters after ``self`` are keyword-only (insert ``*`` immediately + after ``self``). This is **enforced at class-definition time** by + `Scorer.__init_subclass__` calling `enforce_keyword_only_init` + (see `pyrit/common/brick_contract.py`). Non-conforming subclasses + raise `TypeError` at import time. +- ``super().__init__(validator=..., chat_target=...)`` is required so the + base class wires the validator and validates ``TARGET_REQUIREMENTS`` + against any provided ``chat_target``. +- Existing subclasses that cannot adopt the contract immediately may set + the class attribute ``_brick_legacy_init = True`` to opt into a + one-release grace period that downgrades the error to a + ``DeprecationWarning(removed_in="0.16.0")``. The opt-out is removed in + 0.16.0; classes that still violate the contract at that point will hard + fail. + +### Currently grandfathered + +- ``PlagiarismScorer`` (``pyrit/score/float_scale/plagiarism_scorer.py``) — + accepts ``reference_text`` positionally as part of its public API. The + positional shape is preserved through one release cycle via + ``_brick_legacy_init = True`` and is scheduled to become + keyword-only in 0.16.0 (``BREAKING CHANGE``). + +## Common pitfalls + +- Forgetting ``*`` after ``self`` — the new check will surface this at + import time with the exact list of positional parameters that need to be + made keyword-only. +- Calling ``super().__init__`` with positional args — the base + ``Scorer.__init__`` is already keyword-only, so positional calls raise + ``TypeError`` at runtime. Always forward via kwargs. diff --git a/.github/instructions/targets.instructions.md b/.github/instructions/targets.instructions.md new file mode 100644 index 000000000..19040be72 --- /dev/null +++ b/.github/instructions/targets.instructions.md @@ -0,0 +1,111 @@ +--- +applyTo: "pyrit/prompt_target/**" +--- + +# Prompt Target Development Guidelines + +## Base Class Contract + +All targets MUST inherit from ``PromptTarget`` (or one of its public +subclasses such as ``OpenAITarget`` / ``HTTPTarget``) and implement +``_send_prompt_to_target_async``: + +```python +from pyrit.prompt_target.common.prompt_target import PromptTarget + + +class MyTarget(PromptTarget): + def __init__( + self, + *, + endpoint: str, + api_key: str, + max_requests_per_minute: int | None = None, + custom_configuration: TargetConfiguration | None = None, + ) -> None: + super().__init__( + endpoint=endpoint, + max_requests_per_minute=max_requests_per_minute, + custom_configuration=custom_configuration, + ) + self._api_key = api_key + + async def _send_prompt_to_target_async( + self, *, normalized_conversation: list[Message] + ) -> list[Message]: + ... +``` + +``send_prompt_async`` (the public entry point) is ``@final`` and MUST NOT +be overridden. Override ``_send_prompt_to_target_async`` instead. + +## Keyword-only ``__init__`` is enforced + +Every ``PromptTarget`` subclass MUST make all ``__init__`` parameters +keyword-only (i.e., place ``*`` as the first parameter after ``self``). +``PromptTarget.__init_subclass__`` validates this at class-definition time +via ``enforce_keyword_only_init`` and raises ``TypeError`` on violations. + +The check is satisfied by either of: + +```python +def __init__(self, *, endpoint: str, api_key: str) -> None: ... + +def __init__(self, *args: Any, **kwargs: Any) -> None: ... # *args after self +``` + +It rejects: + +```python +def __init__(self, endpoint: str, api_key: str) -> None: ... # missing * +``` + +> [!NOTE] +> ``PromptTarget.__init__`` *itself* still accepts positional parameters and +> is not currently keyword-only. The ``__init_subclass__`` hook only runs for +> subclasses, so the base class non-compliance is tolerated during the warn- +> first phase. The base ``__init__`` will be reshaped to be keyword-only in +> 0.16.0 as a BREAKING CHANGE. + +## Temporary opt-out: ``_brick_legacy_init`` + +A handful of legacy targets whose positional ``__init__`` is part of the +public API are grandfathered with ``_brick_legacy_init = True``. They +emit a ``DeprecationWarning`` at import time and the opt-out is scheduled +for removal in **0.16.0**. Do not set this flag on new targets; new +targets MUST follow the keyword-only contract. + +Currently grandfathered (slated for cleanup in 0.16.0): +``HTTPTarget``, ``OpenAICompletionTarget``, ``OpenAIImageTarget``, +``PromptShieldTarget``. + +## Configuration and Capabilities + +- Set ``_DEFAULT_CONFIGURATION`` at the class level when your target's + capabilities differ from the base defaults (multi-turn support, non-text + modalities, JSON-mode responses, etc.). +- Accept ``custom_configuration: TargetConfiguration | None = None`` in + ``__init__`` and forward it to ``super().__init__`` so callers can + override capabilities per-instance (this is required for HTTP / Playwright + targets whose capabilities depend on deployment configuration). + +## Identifiable Pattern + +All targets inherit ``Identifiable``. Override ``_build_identifier()`` to +include parameters that affect target behaviour: + +```python +def _build_identifier(self) -> ComponentIdentifier: + return self._create_identifier( + params={"endpoint": self._endpoint, "model_name": self._model_name}, + ) +``` + +Include: endpoint, model_name, deployment identifiers, custom headers that +affect routing. +Exclude: API keys, retry counts, logging config, timeouts. + +## Exports + +New targets MUST be added to ``pyrit/prompt_target/__init__.py`` — both +the import and the ``__all__`` list. diff --git a/doc/code/framework.md b/doc/code/framework.md index e52bb4849..76d80a130 100644 --- a/doc/code/framework.md +++ b/doc/code/framework.md @@ -67,7 +67,7 @@ The main components of PyRIT are prompts, attacks, converters, targets, and scor ![alt text](../../assets/architecture_components.png) -As much as possible, each component is a Lego-brick of functionality. Prompts from one attack can be used in another. An attack for one scenario can use multiple targets. And sometimes you completely skip components (e.g. almost every component can be a NoOp also, you can have a NoOp converter that doesn't convert, or a NoOp target that just prints the prompts). +As much as possible, each component is a pluggable brick of functionality. Prompts from one attack can be used in another. An attack for one scenario can use multiple targets. And sometimes you completely skip components (e.g. almost every component can be a NoOp also, you can have a NoOp converter that doesn't convert, or a NoOp target that just prints the prompts). If you are contributing to PyRIT, that work will most likely land in one of these buckets and be as self-contained as possible. It isn't always this clean, but when an attack scenario doesn't quite fit (and that's okay!) it's good to brainstorm with the maintainers about how we can modify our architecture. diff --git a/pyrit/common/__init__.py b/pyrit/common/__init__.py index c1ea318fa..44b4c381a 100644 --- a/pyrit/common/__init__.py +++ b/pyrit/common/__init__.py @@ -20,6 +20,7 @@ reset_default_values, set_default_value, ) +from pyrit.common.brick_contract import enforce_keyword_only_init 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 @@ -41,6 +42,7 @@ "combine_dict", "combine_list", "DefaultValueScope", + "enforce_keyword_only_init", "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 new file mode 100644 index 000000000..fe2a0bd8e --- /dev/null +++ b/pyrit/common/brick_contract.py @@ -0,0 +1,99 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT license. + +""" +Constructor contract enforcement for PyRIT's pluggable brick base classes. + +Several PyRIT base classes (``PromptConverter``, ``Scorer``, ``PromptTarget``, +``Scenario``, ``AttackStrategy``, ``SeedDatasetProvider``) are extension +points that users routinely swap in and out. To make those swaps predictable, +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 either +raise ``TypeError`` at class definition time, or, if they opt in via the +``_brick_legacy_init`` class attribute, emit a ``DeprecationWarning`` +via ``print_deprecation_message`` and continue. +The opt-out is intended to be removed in ``0.16.0``. +""" + +from __future__ import annotations + +import inspect +from inspect import Parameter + +from pyrit.common.deprecation import print_deprecation_message + +#: Class attribute name that opts a subclass into the legacy-init grace period. +#: When ``True`` on a class, ``enforce_keyword_only_init`` downgrades the +#: ``TypeError`` to a ``DeprecationWarning`` until ``_LEGACY_REMOVED_IN``. +LEGACY_INIT_OPT_OUT_ATTR = "_brick_legacy_init" + +#: Version in which the legacy-init opt-out is removed; non-conforming +#: subclasses will hard-fail at that point. +_LEGACY_REMOVED_IN = "0.16.0" + + +def enforce_keyword_only_init(cls: type, *, base_name: str) -> None: + """ + Validate that ``cls.__init__`` only accepts keyword-only parameters. + + Intended to be called from a base class's ``__init_subclass__`` hook to + enforce the brick constructor contract on subclasses. + + The helper only inspects ``__init__`` defined directly on ``cls`` (i.e. + ``"__init__" in cls.__dict__``). Subclasses that inherit ``__init__`` + from their parent are not re-checked — the parent will already have been + checked at its own definition time. + + Args: + cls: The subclass being defined. Pass through from + ``__init_subclass__``. + base_name: Display name of the base class (e.g. ``"Scenario"``). + Used in error messages so the user knows which contract was + violated. + + Raises: + TypeError: If ``cls.__init__`` accepts any positional or + positional-or-keyword parameters after ``self``, and ``cls`` does + not opt into the legacy-init grace period via the + ``_brick_legacy_init`` class attribute. The opt-out is only + honored when set directly on ``cls`` (it is not inherited from a + base class), so new subclasses always get the hard check by + default. + """ + if "__init__" not in cls.__dict__: + # Subclass inherits __init__ from its parent; the parent has already + # been validated. Nothing to check here. + return + + sig = inspect.signature(cls.__init__) + # Skip ``self`` (always the first parameter on an unbound method). + params = list(sig.parameters.values())[1:] + + offenders = [p.name for p in params if p.kind in (Parameter.POSITIONAL_ONLY, Parameter.POSITIONAL_OR_KEYWORD)] + if not offenders: + return + + if cls.__dict__.get(LEGACY_INIT_OPT_OUT_ATTR, False): + # Opt-in legacy period: warn rather than break, so existing users + # whose code calls these constructors positionally have one release + # cycle to migrate. + print_deprecation_message( + old_item=(f"{cls.__module__}.{cls.__qualname__}.__init__ with positional parameters {offenders!r}"), + new_item=(f"keyword-only parameters per the {base_name} contract (insert ``*`` after ``self``)"), + removed_in=_LEGACY_REMOVED_IN, + ) + return + + raise TypeError( + f"{cls.__name__}.__init__ violates the {base_name} contract: " + f"all parameters after ``self`` must be keyword-only, but the " + f"following are positional: {offenders!r}. Insert ``*,`` after " + f"``self`` to fix, or set ``{LEGACY_INIT_OPT_OUT_ATTR} = True`` on " + f"the class to opt into a temporary deprecation period (removed in " + f"{_LEGACY_REMOVED_IN})." + ) diff --git a/pyrit/datasets/seed_datasets/seed_dataset_provider.py b/pyrit/datasets/seed_datasets/seed_dataset_provider.py index 3e27d9e05..4c9127172 100644 --- a/pyrit/datasets/seed_datasets/seed_dataset_provider.py +++ b/pyrit/datasets/seed_datasets/seed_dataset_provider.py @@ -43,9 +43,15 @@ def __init_subclass__(cls, **kwargs: Any) -> None: This is called when a class inherits from SeedDatasetProvider. A deprecation warning is emitted for subclasses that still override the - legacy ``fetch_dataset`` instead of ``fetch_dataset_async``. + legacy ``fetch_dataset`` instead of ``fetch_dataset_async``. The + keyword-only ``__init__`` contract is also enforced via + ``enforce_keyword_only_init``. """ super().__init_subclass__(**kwargs) + # Local import to avoid a circular dependency at package init time. + from pyrit.common.brick_contract import enforce_keyword_only_init + + enforce_keyword_only_init(cls, base_name="SeedDatasetProvider") if not inspect.isabstract(cls) and ( cls.fetch_dataset is not SeedDatasetProvider.fetch_dataset and cls.fetch_dataset_async is SeedDatasetProvider.fetch_dataset_async diff --git a/pyrit/executor/attack/core/attack_strategy.py b/pyrit/executor/attack/core/attack_strategy.py index 887a467a1..333dde19f 100644 --- a/pyrit/executor/attack/core/attack_strategy.py +++ b/pyrit/executor/attack/core/attack_strategy.py @@ -348,12 +348,29 @@ class AttackStrategy(Strategy[AttackStrategyContextT, AttackStrategyResultT], Id """ Abstract base class for attack strategies. Defines the interface for executing attacks and handling results. + + Subclasses must use the keyword-only constructor shape + (``def __init__(self, *, ...)``); the contract is enforced at class + definition time via ``enforce_keyword_only_init``. See + ``.github/instructions/attacks.instructions.md`` for the full contract. """ #: Capability requirements placed on ``objective_target``. Subclasses #: override to declare what the attack needs. Validated in ``__init__``. TARGET_REQUIREMENTS: ClassVar[TargetRequirements] = TargetRequirements() + def __init_subclass__(cls, **kwargs: Any) -> None: + """ + Enforce the keyword-only constructor contract on subclasses. + + See ``.github/instructions/attacks.instructions.md`` for the contract. + """ + super().__init_subclass__(**kwargs) + # Local import to avoid a circular dependency at package init time. + from pyrit.common.brick_contract import enforce_keyword_only_init + + enforce_keyword_only_init(cls, base_name="AttackStrategy") + def __init__( self, *, diff --git a/pyrit/prompt_converter/add_image_to_video_converter.py b/pyrit/prompt_converter/add_image_to_video_converter.py index 148f4d5e0..8b7ce67fa 100644 --- a/pyrit/prompt_converter/add_image_to_video_converter.py +++ b/pyrit/prompt_converter/add_image_to_video_converter.py @@ -35,6 +35,11 @@ class AddImageVideoConverter(PromptConverter): SUPPORTED_INPUT_TYPES = ("image_path",) SUPPORTED_OUTPUT_TYPES = ("video_path",) + # Grandfathered: ``video_path`` is part of the public positional API. + # TODO: remove this opt-out and insert ``*,`` after ``self`` in 0.16.0 + # (this will be a BREAKING CHANGE for callers passing arguments positionally). + _brick_legacy_init = True + def __init__( self, video_path: str, diff --git a/pyrit/prompt_converter/ansi_escape/ansi_attack_converter.py b/pyrit/prompt_converter/ansi_escape/ansi_attack_converter.py index aaee76c92..9096e3437 100644 --- a/pyrit/prompt_converter/ansi_escape/ansi_attack_converter.py +++ b/pyrit/prompt_converter/ansi_escape/ansi_attack_converter.py @@ -31,6 +31,10 @@ class AnsiAttackConverter(PromptConverter): SUPPORTED_INPUT_TYPES = ("text",) SUPPORTED_OUTPUT_TYPES = ("text",) + # Grandfathered: all six boolean flags are part of the public positional API. + # TODO: remove this opt-out and insert ``*,`` after ``self`` in 0.16.0. + _brick_legacy_init = True + def __init__( self, include_raw: bool = True, diff --git a/pyrit/prompt_converter/ascii_art_converter.py b/pyrit/prompt_converter/ascii_art_converter.py index 0ea9fb33a..0a65ddf98 100644 --- a/pyrit/prompt_converter/ascii_art_converter.py +++ b/pyrit/prompt_converter/ascii_art_converter.py @@ -16,6 +16,10 @@ class AsciiArtConverter(PromptConverter): SUPPORTED_INPUT_TYPES = ("text",) SUPPORTED_OUTPUT_TYPES = ("text",) + # Grandfathered: ``font`` is part of the public positional API. + # TODO: remove this opt-out and insert ``*,`` after ``self`` in 0.16.0. + _brick_legacy_init = True + def __init__(self, font: str = "rand") -> None: """ Initialize the converter with a specified font. diff --git a/pyrit/prompt_converter/ask_to_decode_converter.py b/pyrit/prompt_converter/ask_to_decode_converter.py index 847360e37..5542050df 100644 --- a/pyrit/prompt_converter/ask_to_decode_converter.py +++ b/pyrit/prompt_converter/ask_to_decode_converter.py @@ -39,6 +39,11 @@ class AskToDecodeConverter(PromptConverter): all_templates = garak_templates + extra_templates + # Grandfathered: ``template`` and ``encoding_name`` are part of the public + # positional API. + # TODO: remove this opt-out and insert ``*,`` after ``self`` in 0.16.0. + _brick_legacy_init = True + def __init__(self, template: Optional[str] = None, encoding_name: str = "cipher") -> None: """ Initialize the converter with a specified encoding name and template. diff --git a/pyrit/prompt_converter/diacritic_converter.py b/pyrit/prompt_converter/diacritic_converter.py index f43c58406..e3ccb5b97 100644 --- a/pyrit/prompt_converter/diacritic_converter.py +++ b/pyrit/prompt_converter/diacritic_converter.py @@ -18,6 +18,11 @@ class DiacriticConverter(PromptConverter): SUPPORTED_INPUT_TYPES = ("text",) SUPPORTED_OUTPUT_TYPES = ("text",) + # Grandfathered: ``target_chars`` and ``accent`` are part of the public + # positional API. + # TODO: remove this opt-out and insert ``*,`` after ``self`` in 0.16.0. + _brick_legacy_init = True + def __init__(self, target_chars: str = "aeiou", accent: str = "acute") -> None: """ Initialize the converter with specified target characters and diacritic accent. diff --git a/pyrit/prompt_converter/insert_punctuation_converter.py b/pyrit/prompt_converter/insert_punctuation_converter.py index 833f25d12..668917c69 100644 --- a/pyrit/prompt_converter/insert_punctuation_converter.py +++ b/pyrit/prompt_converter/insert_punctuation_converter.py @@ -25,6 +25,11 @@ class InsertPunctuationConverter(PromptConverter): #: Common punctuation characters. Used if no punctuation list is provided. default_punctuation_list = [",", ".", "!", "?", ":", ";", "-"] + # Grandfathered: ``word_swap_ratio`` and ``between_words`` are part of the + # public positional API. + # TODO: remove this opt-out and insert ``*,`` after ``self`` in 0.16.0. + _brick_legacy_init = True + def __init__(self, word_swap_ratio: float = 0.2, between_words: bool = True) -> None: """ Initialize the converter with a word swap ratio and punctuation insertion mode. diff --git a/pyrit/prompt_converter/pdf_converter.py b/pyrit/prompt_converter/pdf_converter.py index 4017f426e..bfb46ffe8 100644 --- a/pyrit/prompt_converter/pdf_converter.py +++ b/pyrit/prompt_converter/pdf_converter.py @@ -36,6 +36,10 @@ class PDFConverter(PromptConverter): SUPPORTED_INPUT_TYPES = ("text",) SUPPORTED_OUTPUT_TYPES = ("binary_path",) + # Grandfathered: all parameters are part of the public positional API. + # TODO: remove this opt-out and insert ``*,`` after ``self`` in 0.16.0. + _brick_legacy_init = True + def __init__( self, prompt_template: Optional[SeedPrompt] = None, diff --git a/pyrit/prompt_converter/prompt_converter.py b/pyrit/prompt_converter/prompt_converter.py index 0e6916f37..98a80e5c2 100644 --- a/pyrit/prompt_converter/prompt_converter.py +++ b/pyrit/prompt_converter/prompt_converter.py @@ -60,16 +60,22 @@ class PromptConverter(Identifiable): def __init_subclass__(cls, **kwargs: object) -> None: """ - Validate that concrete subclasses define required class attributes. + Validate that concrete subclasses define required class attributes + and follow the keyword-only ``__init__`` contract. Args: **kwargs: Additional keyword arguments passed to the superclass. Raises: TypeError: If a concrete subclass does not define non-empty SUPPORTED_INPUT_TYPES - or SUPPORTED_OUTPUT_TYPES. + or SUPPORTED_OUTPUT_TYPES, or if its ``__init__`` accepts + positional parameters after ``self``. """ super().__init_subclass__(**kwargs) + # Local import to avoid a circular dependency at package init time. + from pyrit.common.brick_contract import enforce_keyword_only_init + + enforce_keyword_only_init(cls, base_name="PromptConverter") # Only validate concrete (non-abstract) classes if not inspect.isabstract(cls): if not cls.SUPPORTED_INPUT_TYPES: diff --git a/pyrit/prompt_converter/qr_code_converter.py b/pyrit/prompt_converter/qr_code_converter.py index cc1424d14..aa8db06ed 100644 --- a/pyrit/prompt_converter/qr_code_converter.py +++ b/pyrit/prompt_converter/qr_code_converter.py @@ -15,6 +15,10 @@ class QRCodeConverter(PromptConverter): SUPPORTED_INPUT_TYPES = ("text",) SUPPORTED_OUTPUT_TYPES = ("image_path",) + # Grandfathered: all parameters are part of the public positional API. + # TODO: remove this opt-out and insert ``*,`` after ``self`` in 0.16.0. + _brick_legacy_init = True + def __init__( self, scale: int = 3, diff --git a/pyrit/prompt_converter/random_capital_letters_converter.py b/pyrit/prompt_converter/random_capital_letters_converter.py index e98bd2a73..728c17097 100644 --- a/pyrit/prompt_converter/random_capital_letters_converter.py +++ b/pyrit/prompt_converter/random_capital_letters_converter.py @@ -16,6 +16,10 @@ class RandomCapitalLettersConverter(PromptConverter): SUPPORTED_INPUT_TYPES = ("text",) SUPPORTED_OUTPUT_TYPES = ("text",) + # Grandfathered: ``percentage`` is part of the public positional API. + # TODO: remove this opt-out and insert ``*,`` after ``self`` in 0.16.0. + _brick_legacy_init = True + def __init__(self, percentage: float = 100.0) -> None: """ Initialize the converter with the specified percentage of randomization. diff --git a/pyrit/prompt_converter/search_replace_converter.py b/pyrit/prompt_converter/search_replace_converter.py index 5904a184b..2b335c1f2 100644 --- a/pyrit/prompt_converter/search_replace_converter.py +++ b/pyrit/prompt_converter/search_replace_converter.py @@ -16,6 +16,11 @@ class SearchReplaceConverter(PromptConverter): SUPPORTED_INPUT_TYPES = ("text",) SUPPORTED_OUTPUT_TYPES = ("text",) + # Grandfathered: ``pattern`` and ``replace`` are part of the public + # positional API (often called as ``SearchReplaceConverter(pattern, replace)``). + # TODO: remove this opt-out and insert ``*,`` after ``self`` in 0.16.0. + _brick_legacy_init = True + def __init__(self, pattern: str, replace: str | list[str], regex_flags: int = 0) -> None: """ Initialize the converter with the specified regex pattern and replacement phrase(s). diff --git a/pyrit/prompt_converter/token_smuggling/ascii_smuggler_converter.py b/pyrit/prompt_converter/token_smuggling/ascii_smuggler_converter.py index 5cca9901e..de0dc0580 100644 --- a/pyrit/prompt_converter/token_smuggling/ascii_smuggler_converter.py +++ b/pyrit/prompt_converter/token_smuggling/ascii_smuggler_converter.py @@ -22,6 +22,10 @@ class AsciiSmugglerConverter(SmugglerConverter): [@embracethered2024unicode] """ + # Grandfathered: ``action`` is inherited from SmugglerConverter's public API. + # TODO: remove this opt-out and insert ``*,`` after ``self`` in 0.16.0. + _brick_legacy_init = True + def __init__(self, action: Literal["encode", "decode"] = "encode", unicode_tags: bool = False) -> None: """ Initialize the converter with options for encoding/decoding. diff --git a/pyrit/prompt_converter/token_smuggling/base.py b/pyrit/prompt_converter/token_smuggling/base.py index ed03bfde4..71ca28913 100644 --- a/pyrit/prompt_converter/token_smuggling/base.py +++ b/pyrit/prompt_converter/token_smuggling/base.py @@ -23,6 +23,12 @@ class SmugglerConverter(PromptConverter, abc.ABC): SUPPORTED_INPUT_TYPES = ("text",) SUPPORTED_OUTPUT_TYPES = ("text",) + # Grandfathered: ``action`` is part of the public positional API of every + # SmugglerConverter subclass. + # TODO: remove this opt-out and insert ``*,`` after ``self`` in 0.16.0 + # (this will be a BREAKING CHANGE for callers passing ``action`` positionally). + _brick_legacy_init = True + def __init__(self, action: Literal["encode", "decode"] = "encode") -> None: """ Initialize the converter with options for encoding/decoding. diff --git a/pyrit/prompt_converter/token_smuggling/sneaky_bits_smuggler_converter.py b/pyrit/prompt_converter/token_smuggling/sneaky_bits_smuggler_converter.py index 503c9dab9..dbb7a1e31 100644 --- a/pyrit/prompt_converter/token_smuggling/sneaky_bits_smuggler_converter.py +++ b/pyrit/prompt_converter/token_smuggling/sneaky_bits_smuggler_converter.py @@ -22,6 +22,10 @@ class SneakyBitsSmugglerConverter(SmugglerConverter): - [@embracethered2025sneakybits] """ + # Grandfathered: ``action`` is inherited from SmugglerConverter's public API. + # TODO: remove this opt-out and insert ``*,`` after ``self`` in 0.16.0. + _brick_legacy_init = True + def __init__( self, action: Literal["encode", "decode"] = "encode", diff --git a/pyrit/prompt_converter/token_smuggling/variation_selector_smuggler_converter.py b/pyrit/prompt_converter/token_smuggling/variation_selector_smuggler_converter.py index f70ee4902..34f9d5612 100644 --- a/pyrit/prompt_converter/token_smuggling/variation_selector_smuggler_converter.py +++ b/pyrit/prompt_converter/token_smuggling/variation_selector_smuggler_converter.py @@ -29,6 +29,10 @@ class VariationSelectorSmugglerConverter(SmugglerConverter): visible and hidden content within a single string. """ + # Grandfathered: ``action`` is inherited from SmugglerConverter's public API. + # TODO: remove this opt-out and insert ``*,`` after ``self`` in 0.16.0. + _brick_legacy_init = True + def __init__( self, action: Literal["encode", "decode"] = "encode", diff --git a/pyrit/prompt_target/common/prompt_target.py b/pyrit/prompt_target/common/prompt_target.py index aa13658aa..9605ed444 100644 --- a/pyrit/prompt_target/common/prompt_target.py +++ b/pyrit/prompt_target/common/prompt_target.py @@ -42,6 +42,28 @@ class PromptTarget(Identifiable): # constructor parameter, which takes precedence over the class-level value. _DEFAULT_CONFIGURATION: TargetConfiguration = TargetConfiguration(capabilities=TargetCapabilities()) + def __init_subclass__(cls, **kwargs: object) -> None: + """ + Validate that subclasses follow the keyword-only ``__init__`` contract. + + Args: + **kwargs: Additional keyword arguments passed to the superclass. + + Raises: + TypeError: If the subclass ``__init__`` accepts positional parameters + after ``self`` and is not grandfathered via ``_brick_legacy_init``. + """ + super().__init_subclass__(**kwargs) + # Local import to avoid a circular dependency at package init time. + from pyrit.common.brick_contract import enforce_keyword_only_init + + enforce_keyword_only_init(cls, base_name="PromptTarget") + + # TODO: ``PromptTarget.__init__`` itself accepts positional parameters, which + # violates the keyword-only contract enforced by ``__init_subclass__`` on + # subclasses. The hook only runs for subclasses, so the base class non- + # compliance is tolerated during the warn-first phase. Reshape this + # signature (insert ``*`` after ``self``) in 0.16.0 as a BREAKING CHANGE. def __init__( self, verbose: bool = False, diff --git a/pyrit/prompt_target/http_target/http_target.py b/pyrit/prompt_target/http_target/http_target.py index 6ed37c0da..f1c5988ac 100644 --- a/pyrit/prompt_target/http_target/http_target.py +++ b/pyrit/prompt_target/http_target/http_target.py @@ -32,6 +32,11 @@ class HTTPTarget(PromptTarget): """ + # Grandfathered: ``http_request`` is part of the public positional API. + # TODO: remove this opt-out and insert ``*,`` after ``self`` in 0.16.0 + # (this will be a BREAKING CHANGE for callers passing arguments positionally). + _brick_legacy_init = True + def __init__( self, http_request: str, diff --git a/pyrit/prompt_target/openai/openai_completion_target.py b/pyrit/prompt_target/openai/openai_completion_target.py index 0f43ac5af..9aa04c009 100644 --- a/pyrit/prompt_target/openai/openai_completion_target.py +++ b/pyrit/prompt_target/openai/openai_completion_target.py @@ -21,6 +21,13 @@ class OpenAICompletionTarget(OpenAITarget): _DEFAULT_CONFIGURATION: TargetConfiguration = TargetConfiguration(capabilities=TargetCapabilities()) + # Grandfathered: positional params predate the kwargs-only contract; the + # sandwiched ``*args``/``**kwargs`` shape forwards extras to ``OpenAITarget``. + # TODO: remove this opt-out and move ``*args`` up to immediately after + # ``self`` (or insert ``*,`` and drop ``*args`` entirely) in 0.16.0 + # (this will be a BREAKING CHANGE for callers passing arguments positionally). + _brick_legacy_init = True + def __init__( self, max_tokens: Optional[int] = None, diff --git a/pyrit/prompt_target/openai/openai_image_target.py b/pyrit/prompt_target/openai/openai_image_target.py index 7ae038399..bc56207c7 100644 --- a/pyrit/prompt_target/openai/openai_image_target.py +++ b/pyrit/prompt_target/openai/openai_image_target.py @@ -53,6 +53,13 @@ class OpenAIImageTarget(OpenAITarget): # DALL-E-only quality values that are deprecated in favor of GPT image model values. _DEPRECATED_QUALITY_VALUES = {"standard", "hd"} + # Grandfathered: positional params predate the kwargs-only contract; the + # sandwiched ``*args``/``**kwargs`` shape forwards extras to ``OpenAITarget``. + # TODO: remove this opt-out and move ``*args`` up to immediately after + # ``self`` (or insert ``*,`` and drop ``*args`` entirely) in 0.16.0 + # (this will be a BREAKING CHANGE for callers passing arguments positionally). + _brick_legacy_init = True + def __init__( self, image_size: Literal[ diff --git a/pyrit/prompt_target/prompt_shield_target.py b/pyrit/prompt_target/prompt_shield_target.py index 74c465a91..cb4588b4d 100644 --- a/pyrit/prompt_target/prompt_shield_target.py +++ b/pyrit/prompt_target/prompt_shield_target.py @@ -54,6 +54,12 @@ class PromptShieldTarget(PromptTarget): _api_version: str _force_entry_field: PromptShieldEntryField + # Grandfathered: ``endpoint`` and ``api_key`` are part of the public + # positional API. + # TODO: remove this opt-out and insert ``*,`` after ``self`` in 0.16.0 + # (this will be a BREAKING CHANGE for callers passing arguments positionally). + _brick_legacy_init = True + def __init__( self, endpoint: Optional[str] = None, diff --git a/pyrit/scenario/core/scenario.py b/pyrit/scenario/core/scenario.py index 2238886cd..c23ddf6a1 100644 --- a/pyrit/scenario/core/scenario.py +++ b/pyrit/scenario/core/scenario.py @@ -142,6 +142,11 @@ class Scenario(ABC): # noqa: B024 - retained for subclass type-checking even wi A Scenario represents a comprehensive testing campaign composed of multiple atomic attack tests (AtomicAttacks). It executes each AtomicAttack in sequence and aggregates the results into a ScenarioResult. + + Subclasses must use the keyword-only constructor shape (``def __init__(self, *, ...)``); + the contract is enforced at class-definition time via + ``enforce_keyword_only_init``. See + ``.github/instructions/scenarios.instructions.md`` for the full contract. """ #: Capability requirements placed on ``objective_target``. Subclasses override to declare @@ -156,6 +161,18 @@ class Scenario(ABC): # noqa: B024 - retained for subclass type-checking even wi #: caller-supplied ``include_baseline=True`` raises ``ValueError``. BASELINE_ATTACK_POLICY: ClassVar[BaselineAttackPolicy] = BaselineAttackPolicy.Enabled + def __init_subclass__(cls, **kwargs: Any) -> None: + """ + Enforce the keyword-only constructor contract on subclasses. + + See ``.github/instructions/scenarios.instructions.md`` for the contract. + """ + super().__init_subclass__(**kwargs) + # Local import to avoid a circular dependency at package init time. + from pyrit.common.brick_contract import enforce_keyword_only_init + + enforce_keyword_only_init(cls, base_name="Scenario") + @classmethod def _get_additional_scoring_questions(cls) -> Sequence[Path]: """ diff --git a/pyrit/score/float_scale/plagiarism_scorer.py b/pyrit/score/float_scale/plagiarism_scorer.py index 354776711..c1286532f 100644 --- a/pyrit/score/float_scale/plagiarism_scorer.py +++ b/pyrit/score/float_scale/plagiarism_scorer.py @@ -33,6 +33,15 @@ class PlagiarismScorer(FloatScaleScorer): _DEFAULT_VALIDATOR: ScorerPromptValidator = ScorerPromptValidator(supported_data_types=["text"]) + # Grandfathered: ``reference_text`` is part of the public positional API + # at the time the keyword-only Scorer contract was introduced. Opting + # into the legacy grace period emits a ``DeprecationWarning`` on import + # instead of raising ``TypeError`` so existing user code keeps working + # for one release cycle. TODO: drop this opt-out and insert ``*,`` + # after ``self`` in 0.16.0 (this will be a BREAKING CHANGE for callers + # that still pass parameters positionally). + _brick_legacy_init = True + def __init__( self, reference_text: str, diff --git a/pyrit/score/scorer.py b/pyrit/score/scorer.py index ce133df66..091ecf6c8 100644 --- a/pyrit/score/scorer.py +++ b/pyrit/score/scorer.py @@ -57,6 +57,11 @@ class Scorer(Identifiable, abc.ABC): """ Abstract base class for scorers. + + Subclasses must use the keyword-only constructor shape + (``def __init__(self, *, ...)``); the contract is enforced at class + definition time via ``enforce_keyword_only_init``. See + ``.github/instructions/scorers.instructions.md`` for the full contract. """ # Evaluation configuration - maps input dataset files to a result file. @@ -80,6 +85,18 @@ class Scorer(Identifiable, abc.ABC): #: (Chat Completions API) and ``OpenAIResponseTarget`` (Responses API). score_blocked_content: bool = False + def __init_subclass__(cls, **kwargs: Any) -> None: + """ + Enforce the keyword-only constructor contract on subclasses. + + See ``.github/instructions/scorers.instructions.md`` for the contract. + """ + super().__init_subclass__(**kwargs) + # Local import to avoid a circular dependency at package init time. + from pyrit.common.brick_contract import enforce_keyword_only_init + + enforce_keyword_only_init(cls, base_name="Scorer") + def __init__(self, *, validator: ScorerPromptValidator, chat_target: Optional[PromptTarget] = None) -> None: """ Initialize the Scorer. diff --git a/tests/integration/mocks.py b/tests/integration/mocks.py index 512a4b598..f6ebfe7d6 100644 --- a/tests/integration/mocks.py +++ b/tests/integration/mocks.py @@ -47,7 +47,7 @@ class MockPromptTarget(PromptTarget): prompt_sent: list[str] - def __init__(self, id=None, rpm=None) -> None: # noqa: A002 + def __init__(self, *, id=None, rpm=None) -> None: # noqa: A002 super().__init__(max_requests_per_minute=rpm) self.id = id # noqa: A003 self.prompt_sent = [] diff --git a/tests/unit/common/test_brick_contract.py b/tests/unit/common/test_brick_contract.py new file mode 100644 index 000000000..a845eab4f --- /dev/null +++ b/tests/unit/common/test_brick_contract.py @@ -0,0 +1,168 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT license. + +import warnings + +import pytest + +from pyrit.common.brick_contract import enforce_keyword_only_init + + +class _FakeBase: + """Standalone base class used to drive the helper in isolation.""" + + def __init_subclass__(cls, **kwargs: object) -> None: + super().__init_subclass__(**kwargs) + enforce_keyword_only_init(cls, base_name="_FakeBase") + + +def test_compliant_keyword_only_init_passes() -> None: + class Compliant(_FakeBase): + def __init__(self, *, foo: str, bar: int = 0) -> None: + self.foo = foo + self.bar = bar + + instance = Compliant(foo="hello", bar=3) + assert instance.foo == "hello" + assert instance.bar == 3 + + +def test_self_only_init_passes() -> None: + class SelfOnly(_FakeBase): + def __init__(self) -> None: + pass + + assert SelfOnly() is not None + + +def test_inherited_init_is_not_double_checked() -> None: + """Subclasses without their own __init__ inherit the (compliant) parent.""" + + class Parent(_FakeBase): + def __init__(self, *, foo: str = "") -> None: + self.foo = foo + + class Child(Parent): + pass + + assert Child(foo="x").foo == "x" + + +def test_positional_init_raises_typeerror() -> None: + with pytest.raises(TypeError) as excinfo: + + class Violator(_FakeBase): + def __init__(self, foo: str, bar: int = 0) -> None: + self.foo = foo + self.bar = bar + + message = str(excinfo.value) + assert "_FakeBase contract" in message + assert "foo" in message + assert "bar" in message + assert "_brick_legacy_init" in message + + +def test_positional_or_keyword_default_still_raises() -> None: + """A param with a default is still positional-or-keyword by default.""" + with pytest.raises(TypeError): + + class StillPositional(_FakeBase): + def __init__(self, foo: str = "x") -> None: + self.foo = foo + + +def test_starargs_without_star_marker_raises() -> None: + """``*args`` after positional params doesn't fix the positional params.""" + with pytest.raises(TypeError) as excinfo: + + class StarArgsSandwich(_FakeBase): + def __init__(self, foo: str = "", *args: object) -> None: + self.foo = foo + + assert "foo" in str(excinfo.value) + + +def test_starargs_first_passes() -> None: + """``*args`` immediately after ``self`` makes subsequent params kw-only.""" + + class StarArgsFirst(_FakeBase): + def __init__(self, *args: object, bar: int = 0) -> None: + self.args = args + self.bar = bar + + assert StarArgsFirst(bar=1).bar == 1 + + +def test_legacy_opt_out_downgrades_to_warning() -> None: + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + + class Grandfathered(_FakeBase): + _brick_legacy_init = True + + def __init__(self, foo: str, bar: int = 0) -> None: + self.foo = foo + self.bar = bar + + deprecations = [w for w in caught if issubclass(w.category, DeprecationWarning)] + assert len(deprecations) == 1 + message = str(deprecations[0].message) + assert "Grandfathered" in message + assert "0.16.0" in message + assert "foo" in message + # Class still works after the warning. + instance = Grandfathered("hi", 2) + assert instance.foo == "hi" + assert instance.bar == 2 + + +def test_legacy_opt_out_false_still_raises() -> None: + with pytest.raises(TypeError): + + class NotGrandfathered(_FakeBase): + _brick_legacy_init = False + + def __init__(self, foo: str) -> None: + self.foo = foo + + +def test_legacy_opt_out_is_not_inherited_by_subclass() -> None: + """The opt-out only applies to the class that sets it; subclasses still hard-fail.""" + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + + class Grandfathered(_FakeBase): + _brick_legacy_init = True + + def __init__(self, foo: str) -> None: + self.foo = foo + + deprecations = [w for w in caught if issubclass(w.category, DeprecationWarning)] + assert len(deprecations) == 1 + + with pytest.raises(TypeError) as excinfo: + + class NewSubclass(Grandfathered): + def __init__(self, foo: str, bar: int = 0) -> None: + self.foo = foo + self.bar = bar + + message = str(excinfo.value) + assert "_FakeBase contract" in message + assert "foo" in message + assert "bar" in message + + +def test_error_message_lists_only_positional_offenders() -> None: + """The error message should only list positional offenders, not kw-only ones.""" + with pytest.raises(TypeError) as excinfo: + + class Mixed(_FakeBase): + def __init__(self, positional_one: str, *, keyword_only: int = 0) -> None: + self.positional_one = positional_one + self.keyword_only = keyword_only + + message = str(excinfo.value) + assert "positional_one" in message + assert "keyword_only" not in message diff --git a/tests/unit/mocks.py b/tests/unit/mocks.py index 2d34d455b..9363c8863 100644 --- a/tests/unit/mocks.py +++ b/tests/unit/mocks.py @@ -130,7 +130,7 @@ class MockPromptTarget(PromptTarget): prompt_sent: list[str] - def __init__(self, id=None, rpm=None) -> None: # noqa: A002 + def __init__(self, *, id=None, rpm=None) -> None: # noqa: A002 super().__init__(max_requests_per_minute=rpm) self.id = id self.prompt_sent = [] diff --git a/tests/unit/scenario/core/test_scenario.py b/tests/unit/scenario/core/test_scenario.py index 3a1207fb1..afe9bb42c 100644 --- a/tests/unit/scenario/core/test_scenario.py +++ b/tests/unit/scenario/core/test_scenario.py @@ -138,7 +138,7 @@ class ConcreteScenario(Scenario): # so we don't have to thread include_baseline=False through every initialize_async call. BASELINE_ATTACK_POLICY: ClassVar[BaselineAttackPolicy] = BaselineAttackPolicy.Forbidden - def __init__(self, atomic_attacks_to_return=None, **kwargs): + def __init__(self, *, atomic_attacks_to_return=None, **kwargs): # Add required strategy_class if not provided class TestStrategy(ScenarioStrategy): @@ -685,7 +685,7 @@ def create_mock_truefalse_scorer(): class ConcreteScenarioWithTrueFalseScorer(Scenario): """Concrete implementation of Scenario for testing baseline-only execution.""" - def __init__(self, atomic_attacks_to_return=None, **kwargs): + def __init__(self, *, atomic_attacks_to_return=None, **kwargs): # Add required strategy_class if not provided class TestStrategy(ScenarioStrategy): diff --git a/tests/unit/scenario/core/test_scenario_retry.py b/tests/unit/scenario/core/test_scenario_retry.py index 462ab1a10..53686c58a 100644 --- a/tests/unit/scenario/core/test_scenario_retry.py +++ b/tests/unit/scenario/core/test_scenario_retry.py @@ -167,7 +167,7 @@ class ConcreteScenario(Scenario): BASELINE_ATTACK_POLICY: ClassVar[BaselineAttackPolicy] = BaselineAttackPolicy.Forbidden - def __init__(self, atomic_attacks_to_return=None, objective_scorer=None, **kwargs): + def __init__(self, *, atomic_attacks_to_return=None, objective_scorer=None, **kwargs): strategy_class = kwargs.pop("strategy_class", None) or _build_test_strategy() # Create a default mock scorer if not provided diff --git a/tests/unit/score/test_audio_scorer.py b/tests/unit/score/test_audio_scorer.py index 3b0e51abb..b228e71cb 100644 --- a/tests/unit/score/test_audio_scorer.py +++ b/tests/unit/score/test_audio_scorer.py @@ -21,7 +21,7 @@ class MockTextTrueFalseScorer(TrueFalseScorer): """Mock TrueFalseScorer for testing audio transcription scoring""" - def __init__(self, return_value: bool = True): + def __init__(self, *, return_value: bool = True): self.return_value = return_value validator = ScorerPromptValidator(supported_data_types=["text"]) super().__init__(validator=validator) @@ -48,7 +48,7 @@ async def _score_piece_async(self, message_piece: MessagePiece, *, objective: Op class MockTextFloatScaleScorer(FloatScaleScorer): """Mock FloatScaleScorer for testing audio transcription scoring""" - def __init__(self, return_value: float = 0.8): + def __init__(self, *, return_value: float = 0.8): self.return_value = return_value validator = ScorerPromptValidator(supported_data_types=["text"]) super().__init__(validator=validator) diff --git a/tests/unit/score/test_scorer.py b/tests/unit/score/test_scorer.py index 8996ebdb6..a21e5920a 100644 --- a/tests/unit/score/test_scorer.py +++ b/tests/unit/score/test_scorer.py @@ -1348,7 +1348,7 @@ def true_false_scorer_returns_empty(self, no_valid_pieces_validator): """Create a TrueFalseScorer where _score_piece_async returns empty list.""" class TestTrueFalseScorer(TrueFalseScorer): - def __init__(self, validator): + def __init__(self, *, validator): super().__init__(validator=validator) def _build_identifier(self) -> ComponentIdentifier: @@ -1475,7 +1475,7 @@ def float_scale_scorer_returns_empty(self, no_valid_pieces_validator): from pyrit.score.float_scale.float_scale_scorer import FloatScaleScorer class _TestFloatScaleScorer(FloatScaleScorer): - def __init__(self, validator): + def __init__(self, *, validator): super().__init__(validator=validator) def _build_identifier(self) -> ComponentIdentifier: diff --git a/tests/unit/score/test_true_false_composite_scorer.py b/tests/unit/score/test_true_false_composite_scorer.py index af11b5e04..968b1154e 100644 --- a/tests/unit/score/test_true_false_composite_scorer.py +++ b/tests/unit/score/test_true_false_composite_scorer.py @@ -31,7 +31,7 @@ def _score_aggregator(self, score_list): # Use the AND aggregator from the TrueFalseScoreAggregator class return TrueFalseScoreAggregator.AND(score_list) - def __init__(self, score_value: bool, score_rationale: str, aggregator=None): + def __init__(self, *, score_value: bool, score_rationale: str, aggregator=None): self._score_value = score_value self._score_rationale = score_rationale self.aggregator = aggregator @@ -72,12 +72,12 @@ def mock_request(patch_central_database): @pytest.fixture def true_scorer(patch_central_database): - return MockScorer(True, "This is a true score") + return MockScorer(score_value=True, score_rationale="This is a true score") @pytest.fixture def false_scorer(patch_central_database): - return MockScorer(False, "This is a false score") + return MockScorer(score_value=False, score_rationale="This is a false score") async def test_composite_scorer_and_all_true(mock_request, true_scorer): @@ -195,8 +195,8 @@ def test_get_chat_target_returns_first_available(patch_central_database): """get_chat_target returns the target from the first sub-scorer that has one.""" mock_target = MagicMock() - scorer_without = MockScorer(True, "no target") - scorer_with = MockScorer(True, "has target") + scorer_without = MockScorer(score_value=True, score_rationale="no target") + scorer_with = MockScorer(score_value=True, score_rationale="has target") scorer_with.get_chat_target = MagicMock(return_value=mock_target) composite = TrueFalseCompositeScorer( @@ -208,8 +208,8 @@ def test_get_chat_target_returns_first_available(patch_central_database): def test_get_chat_target_returns_none_when_no_sub_scorer_has_target(patch_central_database): """get_chat_target returns None when no sub-scorer has a chat target.""" - scorer1 = MockScorer(True, "no target") - scorer2 = MockScorer(False, "also no target") + scorer1 = MockScorer(score_value=True, score_rationale="no target") + scorer2 = MockScorer(score_value=False, score_rationale="also no target") composite = TrueFalseCompositeScorer( aggregator=TrueFalseScoreAggregator.OR, diff --git a/tests/unit/score/test_video_scorer.py b/tests/unit/score/test_video_scorer.py index 4c5d4d352..2e5c668ea 100644 --- a/tests/unit/score/test_video_scorer.py +++ b/tests/unit/score/test_video_scorer.py @@ -61,7 +61,7 @@ def video_converter_sample_video(tmp_path, patch_central_database): class MockTrueFalseScorer(TrueFalseScorer): """Mock TrueFalseScorer for testing""" - def __init__(self, return_value: bool = True): + def __init__(self, *, return_value: bool = True): self.return_value = return_value validator = ScorerPromptValidator(supported_data_types=["image_path"]) super().__init__(validator=validator) @@ -93,7 +93,7 @@ async def _score_piece_async(self, message_piece: MessagePiece, *, objective: Op class MockFloatScaleScorer(FloatScaleScorer): """Mock FloatScaleScorer for testing""" - def __init__(self, return_value: float = 0.8): + def __init__(self, *, return_value: float = 0.8): self.return_value = return_value validator = ScorerPromptValidator(supported_data_types=["image_path"]) super().__init__(validator=validator) @@ -285,7 +285,7 @@ def test_video_scorer_default_num_frames(): class MockAudioTrueFalseScorer(TrueFalseScorer): """Mock AudioTrueFalseScorer for testing video+audio integration""" - def __init__(self, return_value: bool = True): + def __init__(self, *, return_value: bool = True): self.return_value = return_value self.received_objective = None # Audio scorer needs to support audio_path data type