Repository navigation
FIX: Stop a scan whose launch scripts would replace a saved entry - #3986
Open
varunj-msft wants to merge 101 commits into
Open
varunj-msft wants to merge 101 commits into
varunj-msft wants to merge 101 commits into
Conversation
…API input External construction now rejects a parameter whose type includes Path unless the calling service passes the name in allowed_path_params after making the value safe. The converter service allows its upload parameters once they are stored, and the target service allows the server-set upload folder and the GitHubCopilotTarget working directory. The scorer-only check is removed, and the "Valid parameters" list leaves out path parameters callers cannot set.
The target catalog now applies the path rule to required parameters too, so a target that needs a disallowed path is left out instead of listed without the field, matching the scorer catalog. allowed_path_params is typed frozenset[str] so a bare string cannot turn the allow-list into a substring match.
The target catalog still lists a target whose required path parameter is its server-set upload folder when the server has one, and leaves it out otherwise.
# Conflicts: # tests/unit/backend/test_converter_service.py
…benchmark's recipes folder
…navailable storage
This was referenced Nov 7, 2026
# Conflicts: # pyrit/registry/registry_recipe_restore.py
# Conflicts: # pyrit/backend/services/runtime_lifecycle.py # tests/unit/backend/test_runtime_lifecycle.py
# Conflicts: # pyrit/setup/initializers/tools.py # tests/unit/backend/test_scenario_run_service.py # tests/unit/backend/test_scenario_suite_execution_service.py # tests/unit/registry/test_attack_technique_registry.py # tests/unit/registry/test_instance_registry.py # tests/unit/registry/test_registry_recipe_restore.py
create_named_instance checks the name before it builds, and the technique registry checks names case-insensitively before it registers. Both raised the duplicate-name error without recording the clash, so a launch script that caught either one could still leave a saved entry in place of its own. The name check now records the clash itself, as register already did, and the technique check records it too.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This is one of the changes for saving Registry entries across restarts. It builds on #3909. Until the earlier PRs merge, the diff here also shows their changes.
Problem. Scripts a scan runs at launch (
initializersonPOST /api/scenarios/runs) register their targets, converters and scorers withreplace=True. One that used the name of an entry restored from a saved recipe replaced that entry for everyone using the app. Scripts that skip a name that's already taken (the technique and tools scripts, round-robin grouping, and the scorer script's preferred scorers) left the saved entry in place, so the scan ran with it instead of their own.Change.
SAVED_ENTRY_METADATA_KEY, holding the kind).DefaultInstanceRegistry.register(..., replace=True)refuses to replace a marked entry withSavedEntryReplacementError. It's aRuntimeError, because the converter and scorer startup scripts skip any entry that raises aValueError.register_from_factories(techniques), the tools script, the target script's round-robin grouping, and the scorer script's two picks by name (the preferred scorer in each category andscale_and_refusal) refuse instead of using a marked entry in place of their own.register, from the name checkcreate_named_instanceruns first, or from the technique registry's case-insensitive name check. A suite shows the same message for that entry instead of the generic one.What stays the same. Startup and apply start from empty registries, run the scripts, then restore, so nothing is marked while the scripts run. Launches whose scripts don't touch a saved name, and scripts replacing startup entries, work as before. Creating entries through the API already refuses taken names.
Not covered. A custom script that changes registry contents without going through the registry methods. Picking a scorer by tag or by metrics still considers every registered scorer, saved ones included.
Tests and Documentation
tests/unit/registry/test_instance_registry.py: replacing a marked entry is refused and changes nothing; a refusal is recorded even when the caller catches it, including a caught name check against a marked entry.tests/unit/registry/test_attack_technique_registry.py:register_from_factoriesrefuses to skip a saved technique and still skips a startup one; a caught case-insensitive clash with a saved technique is recorded.tests/unit/registry/test_converter_registry.py: a caught name clash with a marked entry throughcreate_named_instanceis recorded.tests/unit/registry/test_registry_recipe_restore.py: restore marks the entries it rebuilds, with their kind, and leaves startup entries unmarked.tests/unit/backend/test_scenario_run_service.py: a launch whose script would replace a saved converter returns the conflict and keeps the saved object, for the real converter startup script, a script that raises, a script that catches the error itself, and a script that wraps it in its own error; a script error with no clash passes through unchanged; the tools script stops on a saved technique or converter with its name; a script can still replace a startup entry.tests/unit/setup/test_converter_initializer.py: the converter startup script doesn't swallow the refusal (it would if the error were aValueError).tests/unit/setup/test_scorer_initializer.py: the scorer script refuses to tag a saved scorer under a preferred name or asscale_and_refusal.tests/unit/setup/test_targets_initializer.py: round-robin grouping refuses to leave a saved target under the group's name.tests/unit/backend/test_scenario_suite_execution_service.py: a suite entry shows the clash message.pytest -n 4 --dist=loadfile tests/unit/backend tests/unit/setup tests/unit/registry tests/unit/models: 8,071 passed, 9 skipped, 4 xfailed.No JupyText changes.