Skip to content

FIX: Stop a scan whose launch scripts would replace a saved entry - #3986

Open
varunj-msft wants to merge 86 commits into
microsoft:mainfrom
varunj-msft:varunj-msft/13178-Launch-Keeps-Saved-Entries
Open

varunj-msft wants to merge 86 commits into
microsoft:mainfrom
varunj-msft:varunj-msft/13178-Launch-Keeps-Saved-Entries

Conversation

@varunj-msft

@varunj-msft varunj-msft commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

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 (initializers on POST /api/scenarios/runs) register their targets, converters and scorers with replace=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.

  • Restore marks each entry it rebuilds, in the entry's metadata (SAVED_ENTRY_METADATA_KEY, holding the kind).
  • DefaultInstanceRegistry.register(..., replace=True) refuses to replace a marked entry with SavedEntryReplacementError. It's a RuntimeError, because the converter and scorer startup scripts skip any entry that raises a ValueError.
  • 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 and scale_and_refusal) refuse instead of using a marked entry in place of their own.
  • The launch records refusals while each script runs, so one the script caught itself, or wrapped in an error of its own, still counts, and returns 409 naming the script and the entry. 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.
  • tests/unit/registry/test_attack_technique_registry.py: register_from_factories refuses to skip a saved technique and still skips a startup one.
  • 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 a ValueError).
  • tests/unit/setup/test_scorer_initializer.py: the scorer script refuses to tag a saved scorer under a preferred name or as scale_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.
  • Removing each guard in turn fails at least one of these tests.
  • pytest -n 4 --dist=loadfile tests/unit/backend tests/unit/setup tests/unit/registry tests/unit/models: 8,071 passed, 9 skipped, 4 xfailed.
  • pre-commit (ruff, ty) on the changed files: passed.

No JupyText changes.

…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
The banner tied a retry to its failure only through the failed state, so a
retry from an earlier failure could still show its answer when the polls
missed the runtime becoming ready in between. The generation changes only
when the runtime becomes ready or the backend restarts, so a new one now
starts a new failure too.
A long failure list made the banner taller than a phone screen, and the app
doesn't scroll, so Retry could end up out of reach. The list now scrolls on
its own within a third of the screen, and can be scrolled from the keyboard.
When the list can't be loaded, the banner says so, keeps the last list it had,
and keeps trying.
Scripts a scan runs at launch register their targets, converters and scorers
with replace, so one that used a saved entry's name silently replaced it for
everyone, and scripts that skip names already taken (the technique and tools
scripts, round-robin grouping, the scorer script's preferred scorers) left the
scan with the saved entry instead of their own. Restore now marks the entries
it rebuilds, the registries and those scripts refuse to replace or stand in for
a marked entry, and the launch stops with a 409 naming the script and the entry,
even when the script caught the refusal itself. A suite shows the same message
for that entry.
Copilot AI balanced review requested due to automatic review settings November 5, 2026 15:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

A custom initializer that wraps SavedEntryReplacementError bypasses the recorded-refusal check, causing a 400/500 response instead of the expected 409 and hiding the actionable saved-entry clash message.

1 open issue
What changed in this PR

Protects saved registry entries from replacement or unintended reuse by scan launch scripts. Building on #3909, the stacked changes also add recipe restoration at startup and after live configuration apply.

Changes:

  • Mark restored entries and reject conflicting initializer operations, reporting conflicts to individual scans and suite entries.
  • Add recipe storage, dependency-aware restoration, and runtime failure reporting.
  • Expand regression coverage, isolate test storage, and document configuration.
File Description
tests/​unit/​setup/​test_targets_initializer.py Tests saved-target conflicts during grouping.
tests/​unit/​setup/​test_scorer_initializer.py Tests rejection of saved scorers under preferred names.
tests/​unit/​setup/​test_converter_initializer.py Tests propagation of saved-converter conflicts.
tests/​unit/​setup/​test_configuration_loader.py Tests recipe-source configuration validation.
tests/​unit/​registry/​test_registry_recipe_storage.py Covers recipe parsing and storage diagnostics.
tests/​unit/​registry/​test_registry_recipe_restore.py Covers restoration, dependencies, validation, and saved markers.
tests/​unit/​registry/​test_instance_registry.py Tests replacement protection and refusal recording.
tests/​unit/​registry/​test_file_document_storage.py Covers local and Blob listing failures.
tests/​unit/​registry/​test_attack_technique_registry.py Tests saved-technique conflicts during registration.
tests/​unit/​backend/​test_scenario_suite_execution_service.py Tests actionable suite conflict messages.
tests/​unit/​backend/​test_scenario_run_service.py Tests launch-script conflicts and startup-entry replacement.
tests/​unit/​backend/​test_runtime_lifecycle.py Covers restore readiness, execution context, and recovery.
tests/​unit/​backend/​test_analytics_benchmark.py Checks benchmark recipe-storage isolation.
tests/​unit/​backend/​conftest.py Isolates saved recipes in backend tests.
test_support/​analytics_benchmark.py Configures benchmark-local recipe storage.
pyrit/​setup/​initializers/​tools.py Rejects saved entries under tool component names.
pyrit/​setup/​initializers/​targets.py Rejects saved targets under generated group names.
pyrit/​setup/​initializers/​scorers.py Guards preferred scorer selections by name.
pyrit/​setup/​configuration_loader.py Adds the recipe-storage source setting.
pyrit/​registry/​registry_recipe_storage.py Reads and validates saved recipes.
pyrit/​registry/​registry_recipe_restore.py Restores entries in dependency order and marks them.
pyrit/​registry/​instance_registry.py Blocks saved-entry replacement and records refusals.
pyrit/​registry/​file_document_storage.py Adds document listings with failure diagnostics.
pyrit/​registry/​components/​attack_technique_registry.py Supports saved metadata and guards technique registration.
pyrit/​models/​request_limits.py Adds a shared text-length limit.
pyrit/​models/​catalog/​registry_recipe.py Defines recipe and restore-report models.
pyrit/​models/​catalog/​__init__.py Lazily exports recipe models.
pyrit/​backend/​services/​scenario_suite_execution_service.py Preserves saved-entry conflict messages for suites.
pyrit/​backend/​services/​scenario_run_service.py Converts initializer refusals into launch conflicts.
pyrit/​backend/​services/​runtime_lifecycle.py Restores entries before publishing runtime readiness.
pyrit/​backend/​routes/​scenarios.py Documents launch conflicts in the 409 response.
pyrit/​backend/​README.md Documents offline fixture recipe isolation.
frontend/​e2e/​fixtures/​manual_send_backend.py Uses temporary recipe storage.
doc/​getting_started/​pyrit_conf.md Documents recipe storage and restore-failure recovery.
.pyrit_conf_example Adds recipe-storage configuration guidance.

Review effort: Balanced

@hannahwestra25 hannahwestra25 self-assigned this Nov 5, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants