Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the PyRIT CLI/shell UX by expanding list-targets support (initializers + initialization scripts), tightening argument validation for no-arg commands, fixing help lookup for hyphenated commands, and cleaning up unused CLI plumbing.
Changes:
- Add
list-targetsoption parsing inpyrit_shelland enable--initialization-scriptshandling inpyrit_scan. - Reject unexpected args for commands that should not take arguments and normalize hyphenated command names for
help. - Refactor target listing initialization flow (remove unused
initializer_namesparam) and add unit tests covering new behaviors.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
pyrit/cli/pyrit_shell.py |
Adds arg rejection for no-arg commands; adds option parsing + context override for list-targets; fixes help for hyphenated commands. |
pyrit/cli/pyrit_scan.py |
Resolves --initialization-scripts for --list-targets and surfaces missing-script errors. |
pyrit/cli/frontend_core.py |
Removes unused initializer_names param from list_targets_async and supports initialization-scripts-only population. |
pyrit/cli/_cli_args.py |
Introduces parse_list_targets_arguments for shell-mode parsing. |
pyrit/cli/_banner.py |
Updates banner text to reflect list-targets [opts]. |
tests/unit/cli/test_pyrit_shell.py |
Adds coverage for arg rejection, list-targets option flow, and hyphenated help. |
tests/unit/cli/test_pyrit_scan.py |
Adds coverage for --list-targets with initializers and initialization scripts. |
tests/unit/cli/test_frontend_core.py |
Adds coverage for parse_list_targets_arguments and scripts-only target listing initialization. |
| context_to_use = self._fc.FrontendCore( | ||
| initialization_scripts=resolved_scripts, | ||
| initializer_names=args["initializers"], | ||
| log_level=self.default_log_level, | ||
| ) | ||
| context_to_use._scenario_registry = self.context._scenario_registry | ||
| context_to_use._initializer_registry = self.context._initializer_registry |
There was a problem hiding this comment.
When list-targets is called with options, this creates a new FrontendCore without preserving the shell’s startup configuration (e.g., config_file, database, env_files). That can cause list-targets --initializers/--initialization-scripts to load the default config instead of the one the shell was started with, and it may also lose _silent_reinit (leading to extra env-load output).
Consider building the new context from the existing shell context kwargs (or copying config_file/database/env_files from self.context) and propagating _silent_reinit along with the shared registries.
| context_to_use = self._fc.FrontendCore( | |
| initialization_scripts=resolved_scripts, | |
| initializer_names=args["initializers"], | |
| log_level=self.default_log_level, | |
| ) | |
| context_to_use._scenario_registry = self.context._scenario_registry | |
| context_to_use._initializer_registry = self.context._initializer_registry | |
| frontend_core_kwargs: dict[str, Any] = { | |
| "initialization_scripts": resolved_scripts, | |
| "initializer_names": args["initializers"], | |
| "log_level": self.default_log_level, | |
| } | |
| for attribute_name in ("config_file", "database", "env_files"): | |
| attribute_value = getattr(self.context, attribute_name, None) | |
| if attribute_value is not None: | |
| frontend_core_kwargs[attribute_name] = attribute_value | |
| context_to_use = self._fc.FrontendCore(**frontend_core_kwargs) | |
| context_to_use._scenario_registry = self.context._scenario_registry | |
| context_to_use._initializer_registry = self.context._initializer_registry | |
| context_to_use._silent_reinit = getattr(self.context, "_silent_reinit", False) |
| @@ -197,17 +197,19 @@ def main(args: Optional[list[str]] = None) -> int: | |||
|
|
|||
| if parsed_args.list_targets: | |||
| # Need initializers to populate target registry | |||
There was a problem hiding this comment.
This comment now says initializers are needed to populate the target registry, but --initialization-scripts are also supported in this code path. Updating the comment will avoid misleading future maintainers about the accepted inputs for --list-targets.
| # Need initializers to populate target registry | |
| # Need initializers or initialization scripts to populate the target registry |
Description
This PR makes minor updates to improve the CLI experience, including the following:
list-targetslist_targetslist-scenarios)help commanddid not work properly for hyphenated commands such aslist-targetsorlist-initializersinitializer_namesinlist_targets_asyncTests and Documentation
Unit tests added to ensure behavior of all of the above changes