diff --git a/pyrit/cli/_cli_args.py b/pyrit/cli/_cli_args.py index 1218ba4dc3..891ca7dea3 100644 --- a/pyrit/cli/_cli_args.py +++ b/pyrit/cli/_cli_args.py @@ -621,7 +621,7 @@ def parse_run_arguments(*, args_string: str, declared_params: list[Parameter] | augmented_specs: list[_ArgSpec] = list(_RUN_ARG_SPECS) if declared_params: scenario_specs = [_arg_spec_from_parameter(param=p) for p in declared_params] - _validate_scenario_flag_collisions(scenario_specs=scenario_specs, base_specs=_RUN_ARG_SPECS) + scenario_specs = _resolve_scenario_flag_collisions(scenario_specs=scenario_specs, base_specs=_RUN_ARG_SPECS) augmented_specs.extend(scenario_specs) result = _parse_shell_arguments(parts=parts[1:], arg_specs=augmented_specs) @@ -707,29 +707,44 @@ def parser(raw: str) -> Any: ) -def _validate_scenario_flag_collisions(*, scenario_specs: list[_ArgSpec], base_specs: list[_ArgSpec]) -> None: +def _resolve_scenario_flag_collisions(*, scenario_specs: list[_ArgSpec], base_specs: list[_ArgSpec]) -> list[_ArgSpec]: """ - Reject scenario-vs-built-in and scenario-vs-scenario flag collisions. + Drop scenario specs that collide with a built-in flag; reject scenario-vs-scenario collisions. + + A scenario's ``supported_parameters`` (fetched from the API) include the framework's common + parameters — e.g. ``memory_labels``, ``max_concurrency``, ``max_retries`` — which normalize to + flags already provided as built-ins. Those built-ins own the flag, so the scenario copy is + silently dropped, mirroring ``pyrit_scan._add_scenario_params_from_api``. A genuine + scenario-vs-scenario collision (two declared params normalizing to the same flag) is still a + scenario bug, so it raises. + + Args: + scenario_specs: Specs built from scenario-declared parameters. + base_specs: Built-in run argument specs. + + Returns: + list[_ArgSpec]: The scenario specs whose flags do not collide with a built-in flag. Raises: - ValueError: If a scenario flag duplicates a built-in flag, or two - scenario parameters normalize to the same CLI flag. + ValueError: If two scenario parameters normalize to the same CLI flag. """ base_flags = {flag for spec in base_specs for flag in spec.flags} + resolved: list[_ArgSpec] = [] seen: set[str] = set() for spec in scenario_specs: + # Scenario specs carry a single flag, but guard defensively in case that changes. + if any(flag in base_flags for flag in spec.flags): + # Common framework parameter already handled by the matching built-in flag. + continue for flag in spec.flags: - if flag in base_flags: - raise ValueError( - f"Scenario parameter flag {flag!r} collides with a built-in flag. " - f"Rename the parameter to avoid the collision." - ) if flag in seen: raise ValueError( f"Scenario declares two parameters that normalize to the same CLI flag {flag!r}. " f"Rename one of them." ) seen.add(flag) + resolved.append(spec) + return resolved def extract_scenario_args(*, parsed: dict[str, Any]) -> dict[str, Any]: diff --git a/tests/unit/cli/test_cli_args.py b/tests/unit/cli/test_cli_args.py index c8e2cab090..8224675b87 100644 --- a/tests/unit/cli/test_cli_args.py +++ b/tests/unit/cli/test_cli_args.py @@ -3,7 +3,22 @@ import pytest -from pyrit.cli._cli_args import _argparse_validator +from pyrit.cli._cli_args import _argparse_validator, parse_run_arguments +from pyrit.models import Parameter + + +def _sp(*, name: str, description: str = "", param_type: str = "str") -> Parameter: + """Build a real Parameter from Summary-style kwargs (param_type as a string).""" + return Parameter.model_validate( + { + "name": name, + "description": description, + "default": None, + "type_name": param_type, + "choices": None, + "is_list": False, + } + ) def test_argparse_validator_no_params_raises(): @@ -23,3 +38,38 @@ def validate_name(*, name: str) -> str: wrapped = _argparse_validator(validate_name) assert wrapped("hello") == "HELLO" + + +def test_parse_run_arguments_skips_scenario_params_colliding_with_builtins(): + """ + Scenario ``supported_parameters`` include framework common params (e.g. memory_labels) + whose flags match built-ins. Those must be silently dropped, not raise, so ``run`` works. + """ + declared_params = [ + _sp(name="memory_labels", description="Common framework param."), + _sp(name="max_concurrency", description="Common framework param.", param_type="int"), + _sp(name="max_turns", description="Scenario custom param.", param_type="int"), + ] + + result = parse_run_arguments( + args_string="airt.scam --target openai_chat --max-turns 3", + declared_params=declared_params, + ) + + assert result["scenario_name"] == "airt.scam" + assert result["target"] == "openai_chat" + # The dropped common params keep their built-in result keys (not scenario__-prefixed). + assert result["scenario__max_turns"] == 3 + assert "scenario__memory_labels" not in result + assert "scenario__max_concurrency" not in result + + +def test_parse_run_arguments_raises_on_scenario_vs_scenario_collision(): + """Two scenario params normalizing to the same CLI flag remain a scenario bug and raise.""" + declared_params = [ + _sp(name="max_turns", description="First."), + _sp(name="max-turns", description="Normalizes to the same flag."), + ] + + with pytest.raises(ValueError, match="normalize to the same CLI flag"): + parse_run_arguments(args_string="airt.scam", declared_params=declared_params)