Skip to content

Commit dfb10a7

Browse files
rlundeen2Copilot
andcommitted
MAINT: Restore scenario-declared CLI parameters (pyrit_scan + pyrit_shell)
The frontend refactor regressed scenario-declared parameter handling on both CLI entry points - flags like `--max-turns 7` were rejected before the second pass could fetch metadata from the server. pyrit_scan - `parse_args` now uses `parse_known_args` (pass 1 is tolerant of scenario-specific flags) and stashes leftovers + the raw arg list on the Namespace for the second pass. - `_reparse_with_scenario_params` threads the original `args` list instead of reading `sys.argv[1:]`, so explicit-args callers work. - `main` does a strict re-parse when there are unknown args but no scenario is specified, preserving the original `exit 2` error for truly invalid flags. pyrit_shell - `do_run` now fetches `get_scenario_async` first, builds `Parameter` objects from the response's `supported_parameters`, and threads `declared_params` into `parse_run_arguments`. - Calls `extract_scenario_args` and propagates `scenario_params` on the REST request payload. Tests - New `TestScenarioParamFlow` (4 tests) and `TestShellScenarioParamFlow` (4 tests) regression cases covering forward, invalid-flag, no-params, and metadata-fetch failure paths. Diff coverage remains at 96%. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 8fa7299 commit dfb10a7

4 files changed

Lines changed: 190 additions & 8 deletions

File tree

pyrit/cli/pyrit_scan.py

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -283,8 +283,14 @@ def _extract_scenario_args(*, parsed: Namespace) -> dict[str, Any]:
283283

284284
def parse_args(args: Optional[list[str]] = None) -> Namespace:
285285
"""
286-
Parse command-line arguments (pass 1 only — scenario-specific flags
287-
are added via a second parse after fetching scenario metadata from server).
286+
Parse command-line arguments (pass 1 — tolerant of scenario-declared flags).
287+
288+
Pass 1 uses ``parse_known_args`` so scenario-specific flags (e.g.
289+
``--max-turns 7``) don't cause an error before we've had a chance to
290+
fetch the scenario's declared parameters from the server. The unknown
291+
leftovers are stashed on the returned Namespace as ``_unknown_args``
292+
so :func:`_reparse_with_scenario_params` can detect truly unknown flags
293+
when no scenario was specified.
288294
289295
Args:
290296
args: Argument list (``sys.argv[1:]`` when None).
@@ -293,7 +299,10 @@ def parse_args(args: Optional[list[str]] = None) -> Namespace:
293299
Namespace: Parsed command-line arguments.
294300
"""
295301
parser = _build_base_parser(add_help=True)
296-
return parser.parse_args(args)
302+
parsed, unknown = parser.parse_known_args(args)
303+
parsed._unknown_args = unknown
304+
parsed._raw_args = list(args) if args is not None else list(sys.argv[1:])
305+
return parsed
297306

298307

299308
async def _resolve_server_url_async(*, parsed_args: Namespace) -> str | None:
@@ -443,17 +452,32 @@ def _reparse_with_scenario_params(
443452
*, parsed_args: Namespace, supported_params: list[dict[str, Any]]
444453
) -> Namespace | None:
445454
"""
446-
Re-parse ``sys.argv`` with scenario-declared flags added to the base parser.
455+
Re-parse the original args with scenario-declared flags added to the base parser.
456+
457+
The original argument list is read from ``parsed_args._raw_args`` (populated
458+
by :func:`parse_args`). If no scenario-declared parameters are supplied but
459+
pass 1 left unknown args behind, surface the error now via strict re-parse.
447460
448461
Returns:
449462
Namespace | None: The re-parsed Namespace, or ``None`` on argparse ``SystemExit``.
450463
"""
464+
raw_args: list[str] = getattr(parsed_args, "_raw_args", sys.argv[1:] if len(sys.argv) > 1 else [])
465+
451466
if not supported_params:
452-
return parsed_args
467+
unknown = getattr(parsed_args, "_unknown_args", None)
468+
if not unknown:
469+
return parsed_args
470+
# Re-parse strictly so argparse prints the standard "unrecognized arguments" error
471+
strict_parser = _build_base_parser(add_help=True)
472+
try:
473+
return strict_parser.parse_args(raw_args)
474+
except SystemExit:
475+
return None
476+
453477
pass2_parser = _build_base_parser(add_help=True)
454478
_add_scenario_params_from_api(parser=pass2_parser, params=supported_params)
455479
try:
456-
return pass2_parser.parse_args(sys.argv[1:] if len(sys.argv) > 1 else [])
480+
return pass2_parser.parse_args(raw_args)
457481
except SystemExit:
458482
return None
459483

@@ -682,6 +706,16 @@ def main(args: Optional[list[str]] = None) -> int:
682706
except SystemExit as e:
683707
return e.code if isinstance(e.code, int) else 1
684708

709+
# If there are leftover unknown flags AND no scenario was specified,
710+
# there's no chance for pass 2 to recognize them - fail loudly now.
711+
unknown = getattr(parsed_args, "_unknown_args", [])
712+
if unknown and not parsed_args.scenario_name:
713+
strict_parser = _build_base_parser(add_help=True)
714+
try:
715+
strict_parser.parse_args(parsed_args._raw_args)
716+
except SystemExit as e:
717+
return e.code if isinstance(e.code, int) else 1
718+
685719
logging.basicConfig(level=parsed_args.log_level)
686720

687721
return asyncio.run(_run_async(parsed_args=parsed_args))

pyrit/cli/pyrit_shell.py

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -242,16 +242,40 @@ def do_run(self, line: str) -> None:
242242
print("Usage: run <scenario_name> --target <name> [options]")
243243
return
244244

245-
from pyrit.cli._cli_args import parse_run_arguments
245+
from pyrit.cli._cli_args import extract_scenario_args, parse_run_arguments
246246
from pyrit.cli._output import (
247247
print_scenario_result_async,
248248
print_scenario_run_progress,
249249
print_scenario_run_summary,
250250
)
251+
from pyrit.common.parameter import Parameter
252+
253+
# Fetch scenario metadata so the parser recognizes scenario-declared flags.
254+
scenario_name_token = line.split(maxsplit=1)[0]
255+
declared_params: list[Parameter] | None = None
256+
try:
257+
scenario_meta = asyncio.run(self._api_client.get_scenario_async(scenario_name=scenario_name_token))
258+
except Exception as exc:
259+
print(f"Error fetching scenario metadata: {exc}")
260+
return
261+
if scenario_meta is None:
262+
print(f"Error: Scenario '{scenario_name_token}' not found on server.")
263+
return
264+
supported = scenario_meta.get("supported_parameters") or []
265+
if supported:
266+
declared_params = [
267+
Parameter(
268+
name=p["name"],
269+
description=p.get("description", ""),
270+
param_type=str,
271+
default=p.get("default"),
272+
)
273+
for p in supported
274+
]
251275

252276
# Parse arguments
253277
try:
254-
args = parse_run_arguments(args_string=line, declared_params=None)
278+
args = parse_run_arguments(args_string=line, declared_params=declared_params)
255279
except ValueError as e:
256280
print(f"Error: {e}")
257281
return
@@ -294,6 +318,10 @@ def do_run(self, line: str) -> None:
294318
if args.get("memory_labels"):
295319
request["labels"] = args["memory_labels"]
296320

321+
scenario_params = extract_scenario_args(parsed=args)
322+
if scenario_params:
323+
request["scenario_params"] = scenario_params
324+
297325
# Start run
298326
total_strategies = len(request.get("strategies") or [])
299327
print(f"\nRunning scenario: {scenario_name}")

tests/unit/cli/test_pyrit_scan.py

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,3 +623,75 @@ def test_main_add_initializer_server_disabled(self, mock_client_class, _mock_pro
623623
result = pyrit_scan.main(["--add-initializer", str(script)])
624624
assert result == 1
625625
assert "disabled" in capsys.readouterr().out
626+
627+
628+
class TestScenarioParamFlow:
629+
"""Regression tests for scenario-declared parameters flowing through the CLI."""
630+
631+
@staticmethod
632+
def _build_mock_client(supported_params=None, status="COMPLETED"):
633+
from unittest.mock import AsyncMock
634+
635+
client = AsyncMock()
636+
client.list_scenarios_async.return_value = {"items": [{"scenario_name": "foo"}]}
637+
client.get_scenario_async.return_value = {
638+
"scenario_name": "foo",
639+
"supported_parameters": supported_params or [],
640+
}
641+
client.start_scenario_run_async.return_value = {"scenario_result_id": "rid", "status": "CREATED"}
642+
client.get_scenario_run_async.return_value = {"scenario_result_id": "rid", "status": status}
643+
client.get_scenario_run_results_async.return_value = {"items": []}
644+
client.close_async = AsyncMock()
645+
client.__aenter__ = AsyncMock(return_value=client)
646+
client.__aexit__ = AsyncMock(return_value=None)
647+
return client
648+
649+
@patch("pyrit.cli._server_launcher.ServerLauncher.probe_health_async", new_callable=AsyncMock, return_value=True)
650+
@patch("pyrit.cli.api_client.PyRITApiClient")
651+
@patch("pyrit.cli._output.print_scenario_result_async", new_callable=AsyncMock)
652+
@patch("pyrit.cli._output.print_scenario_run_progress")
653+
def test_scenario_declared_flag_is_forwarded(self, _mock_prog, _mock_print, mock_client_class, _mock_probe):
654+
client = self._build_mock_client(supported_params=[{"name": "max_turns", "description": "..."}])
655+
mock_client_class.return_value = client
656+
657+
result = pyrit_scan.main(["foo", "--target", "t", "--max-turns", "7"])
658+
659+
assert result == 0
660+
sent_request = client.start_scenario_run_async.call_args.kwargs["request"]
661+
assert sent_request["scenario_params"] == {"max_turns": "7"}
662+
663+
@patch("pyrit.cli._server_launcher.ServerLauncher.probe_health_async", new_callable=AsyncMock, return_value=True)
664+
@patch("pyrit.cli.api_client.PyRITApiClient")
665+
@patch("pyrit.cli._output.print_scenario_result_async", new_callable=AsyncMock)
666+
@patch("pyrit.cli._output.print_scenario_run_progress")
667+
def test_unknown_flag_after_valid_scenario_errors(
668+
self, _mock_prog, _mock_print, mock_client_class, _mock_probe
669+
):
670+
client = self._build_mock_client(supported_params=[{"name": "max_turns", "description": "..."}])
671+
mock_client_class.return_value = client
672+
673+
result = pyrit_scan.main(["foo", "--target", "t", "--max-turns", "7", "--unknown-flag"])
674+
675+
assert result == 1
676+
client.start_scenario_run_async.assert_not_called()
677+
678+
@patch("pyrit.cli._server_launcher.ServerLauncher.probe_health_async", new_callable=AsyncMock, return_value=True)
679+
@patch("pyrit.cli.api_client.PyRITApiClient")
680+
@patch("pyrit.cli._output.print_scenario_result_async", new_callable=AsyncMock)
681+
@patch("pyrit.cli._output.print_scenario_run_progress")
682+
def test_no_scenario_params_passes_through_cleanly(self, _mock_prog, _mock_print, mock_client_class, _mock_probe):
683+
client = self._build_mock_client(supported_params=[])
684+
mock_client_class.return_value = client
685+
686+
result = pyrit_scan.main(["foo", "--target", "t"])
687+
688+
assert result == 0
689+
sent_request = client.start_scenario_run_async.call_args.kwargs["request"]
690+
assert "scenario_params" not in sent_request
691+
692+
def test_parse_args_tolerates_scenario_specific_flags(self):
693+
# Pass 1 must not error on scenario-declared flags (they're recognized in pass 2).
694+
parsed = pyrit_scan.parse_args(["foo", "--target", "t", "--max-turns", "7"])
695+
assert parsed.scenario_name == "foo"
696+
assert parsed.target == "t"
697+
assert parsed._unknown_args == ["--max-turns", "7"]

tests/unit/cli/test_pyrit_shell.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ def mock_api_client():
2121
client.list_initializers_async.return_value = {"items": [], "pagination": {"total": 0}}
2222
client.list_targets_async.return_value = {"items": [], "pagination": {"total": 0}}
2323
client.list_scenario_runs_async.return_value = {"items": []}
24+
# Default: scenario fetch returns no declared params (back-compat for older tests)
25+
client.get_scenario_async.return_value = {"scenario_name": "foo", "supported_parameters": []}
2426
client.close_async = AsyncMock()
2527
client.__aenter__ = AsyncMock(return_value=client)
2628
client.__aexit__ = AsyncMock(return_value=None)
@@ -607,3 +609,49 @@ def test_stop_server_close_client_swallows_errors(self, shell):
607609
client.close_async = AsyncMock(side_effect=RuntimeError("ignored"))
608610
s.do_stop_server("")
609611
assert s._api_client is None
612+
613+
614+
class TestShellScenarioParamFlow:
615+
"""Regression tests: shell.do_run must forward scenario-declared parameters."""
616+
617+
def test_run_passes_scenario_declared_params(self, shell):
618+
s, client = shell
619+
client.get_scenario_async.return_value = {
620+
"scenario_name": "foo",
621+
"supported_parameters": [{"name": "max_turns", "description": "..."}],
622+
}
623+
client.start_scenario_run_async = AsyncMock(return_value={"scenario_result_id": "rid", "status": "CREATED"})
624+
client.get_scenario_run_async = AsyncMock(return_value={"scenario_result_id": "rid", "status": "COMPLETED"})
625+
client.get_scenario_run_results_async = AsyncMock(return_value={"items": []})
626+
627+
with (
628+
patch("pyrit.cli._output.print_scenario_result_async", new_callable=AsyncMock),
629+
patch("pyrit.cli._output.print_scenario_run_progress"),
630+
patch("time.sleep"),
631+
):
632+
s.do_run("foo --target t --max-turns 7")
633+
634+
sent_request = client.start_scenario_run_async.call_args.kwargs["request"]
635+
assert sent_request["scenario_params"] == {"max_turns": "7"}
636+
637+
def test_run_metadata_fetch_failure_aborts(self, shell, capsys):
638+
s, client = shell
639+
client.get_scenario_async = AsyncMock(side_effect=RuntimeError("net down"))
640+
s.do_run("foo --target t")
641+
assert "Error fetching scenario metadata" in capsys.readouterr().out
642+
643+
def test_run_unknown_scenario_aborts(self, shell, capsys):
644+
s, client = shell
645+
client.get_scenario_async.return_value = None
646+
s.do_run("foo --target t")
647+
assert "not found on server" in capsys.readouterr().out
648+
649+
def test_run_unknown_flag_for_scenario_with_declared_params_errors(self, shell, capsys):
650+
s, client = shell
651+
client.get_scenario_async.return_value = {
652+
"scenario_name": "foo",
653+
"supported_parameters": [{"name": "max_turns", "description": "..."}],
654+
}
655+
s.do_run("foo --target t --not-a-real-flag x")
656+
captured = capsys.readouterr().out
657+
assert "Unknown argument" in captured or "Error" in captured

0 commit comments

Comments
 (0)