Skip to content

FEAT: Add ScenarioPreset - #2931

Open
hannahwestra25 wants to merge 17 commits into
microsoft:mainfrom
hannahwestra25:scenario-preset-model-and-storage
Open

hannahwestra25 wants to merge 17 commits into
microsoft:mainfrom
hannahwestra25:scenario-preset-model-and-storage

Conversation

@hannahwestra25

@hannahwestra25 hannahwestra25 commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

Description

First PR of the composite scan work. Design doc: Composite Scans in PyRIT.

A scenario preset is a named, reusable, target-agnostic answer to what to test: a scenario plus its configuration. It excludes everything environment-specific — target, concurrency, retries, labels — which belongs to a launch, so one preset runs unchanged against dev, staging, and prod.

This PR covers the full vertical slice. It originally shipped the persistence layer alone; the API and UI were folded in so the storage contract is reviewed against something that exercises it.

Layer Component Responsibility
Model ScenarioPreset The model
StoredPreset A preset paired with the version of the document it came from
Storage ScenarioPresetStorage JSON persistence with optimistic concurrency
FileDocumentStorage Shared local-dir + Blob base: I/O, naming, versions, conditional writes
API /api/scenario-presets CRUD plus POST /{name}/resolve
ScenarioPresetService Validation against the live registry; preset → run request merge
UI ScenarioPresetLibrary Lists presets, surfaces issues, launches, deletes
ScenarioPresetEditor Creates and edits the scenario-owned fields
LaunchPresetDialog Restates what the preset pins, collects the launch-owned fields, starts the run

No built-in presets: RegisteredScenario already ships defaults for five of the seven preset fields, so a built-in would mostly restate them. Deferred to the PR that needs one.

Screenshots

Screenshot 2026-10-02 163613 Screenshot 2026-10-02 163642 Screenshot 2026-10-02 164006 Screenshot 2026-10-02 164021

Worth reviewing

  • Fields are tri-state. None means "use the scenario default", distinct from an explicit equal value. Unknown fields are rejected, not dropped, so a misspelled key fails loudly.
  • An unset field survives an unrelated edit. A scenario-owned field is emitted only if the preset already pinned it or the operator moved it off the scenario default, so fixing a typo in a description doesn't freeze every default the preset was tracking.
  • Presets are held to the request limits a run is held to. ScenarioPreset reuses the annotated types from RunScenarioRequest, so an oversized preset fails at save rather than at launch.
  • The version describes the document, not the preset. It hashes the bytes on disk, so hand-edits are caught. expected_version is separate: None creates, a token updates, and a client-supplied version never reaches storage.
  • Name validation sits at the FileDocumentStorage base, so no document API can address a path outside its source. Listing applies the rule before reading and skips what it can't decode, so one bad file can't hide the rest.
  • Atomicity and conditional writes solve different problems, so both are enforced. os.replace from a sibling temp file stops a reader seeing a half-written document; it does nothing to stop a second writer discarding the first's edit. Each backend keeps that comparison and the write inseparable: Blob creates use overwrite=False and updates send If-Match on the ETag just read; local writes hold an exclusive sibling lock across the whole read-compare-replace. A hand-edit outside the class is caught by the content hash instead.
  • FileDocumentStorage is extracted, not duplicated. CustomInitializerStorage becomes a small subclass with an unchanged public API, so its existing tests are the regression check. Preset storage's separate sidecar lock was folded into the same base, leaving scenario_preset_storage.py a thin adapter.
  • Unresolvable references are advisory, not fatal. A preset from another deployment stores and lists fine, shows what's missing, and has Launch disabled, rather than being rejected at save.
  • Resolve happens server-side. POST /{name}/resolve merges the preset with the launch-owned fields into an ordinary RunScenarioRequest, so unset-vs-set-to-the-default has one implementation.
  • Scenario-owned configuration is shared, not duplicated. scenarioConfigForm, ScenarioTechniqueSelector, and ScenarioDatasetFields come out of ScenarioDetail, so the launch form and the preset editor build the same config from one place.
  • Mutations require an admin; reads are open. Presets are data, not executable code like custom initializers, so there is no allow_ switch.

Tests and Documentation

  • Backend storage — model, preset storage on both backends, and the shared base's listing, decode, and durability contracts through CustomInitializerStorage: tri-state round-trips, version conflicts, lost-update races on both backends, stale-lock reclamation, lock files never surfacing as documents, out-of-band edits and deletes, illegal names, unknown fields, malformed-file skipping, and the previous document surviving a failed write.
  • Backend API — route authorization, 404/409/503 handling, advisory issue reporting, and the preset → RunScenarioRequest merge.
  • Frontend — library issue badges and a disabled Launch naming the missing reference, create vs. update, version-conflict and permission errors, the tri-state form round-trip, the launch dialog's default-vs-pinned summary, and route ranking that keeps /scanner/presets/new from shadowing a preset named new.

Validation: npm run lint and npx tsc --noEmit pass. jest src/components/Scenarios src/components/ScenarioPresets is 268 green across 12 suites, pytest tests/unit/registry tests/unit/models is 2423 green, and the backend preset and lifecycle tests are 107 green. The branch is merged current with main; full-suite coverage is left to CI.

No docs or notebook changes: the preset surface is reached from the scanner UI rather than a notebook, so JupyText was not run.

A scenario preset is a named, reusable, target-agnostic answer to *what to
test*: a scenario plus its scenario-owned configuration. It deliberately
excludes everything environment-specific (target, concurrency, retries,
labels), which belongs to a launch. That split is what lets one preset run
unchanged against dev, staging, and production.

This is the first PR of the composite scan work and adds only the persistence
layer. No API routes, no UI, and no built-in presets are registered yet.

- `ScenarioPreset` / `ScenarioPresetProvenance` models. Every configurable
  field is tri-state: `None` means "not set by this preset, use the scenario
  default", which is distinct from an explicit value that happens to equal
  that default. Collapsing the two would pin a scenario default at save time
  and stop it tracking upstream changes.
- `ScenarioPresetStorage` for JSON persistence with an optimistic-concurrency
  check on `version`. The caller states intent through a separate
  `expected_version` argument rather than through the version on the submitted
  model, so create and update are never ambiguous and a client-supplied
  version is never trusted into storage.
- `ScenarioPresetRegistry` unioning built-in presets (registered from
  initializer code, read-only) with user presets from storage. Built-in wins a
  name collision and the colliding user preset is skipped with a warning, so
  shipping a new built-in cannot break a running install.
- `ScenarioPresetConflictError` (HTTP 409) carrying both versions.
- `FileDocumentStorage`, extracted from `CustomInitializerStorage`, holding the
  shared local-directory and Azure Blob handling for flat named documents.
  Extracted rather than duplicated because a second copy would mean two
  implementations of SAS detection, credential lifecycle, and container-URL
  parsing. `CustomInitializerStorage` now subclasses it with its public API
  unchanged, so its existing tests cover the refactor.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@hannahwestra25
hannahwestra25 force-pushed the scenario-preset-model-and-storage branch from 122cf40 to 77825c6 Compare October 1, 2026 20:27
Copilot AI added 2 commits October 1, 2026 16:59
Addresses review feedback on the preset storage contract.

- Validate registry names in every single-document operation on
  FileDocumentStorage, closing a path-traversal hole that let a preset name
  containing separators read, overwrite, or delete files outside the
  configured source. The check lives at the base so no document API can omit
  it, which hardens CustomInitializerStorage as well.
- Replace the in-model monotonic version counter with an opaque token derived
  from the stored bytes, paired with the preset as StoredPreset. The counter
  lived inside the document it guarded, so a hand-edit rewrote the very value
  used to detect that edit. Hashing the raw content also makes save_preset
  refuse to create over a malformed file instead of clobbering it.
- Make ScenarioPresetRegistry read user presets through to storage instead of
  caching them, removing staleness across processes and the non-atomic cache
  rebind. load_stored_presets() is gone; get_stored_preset() replaces it.
- Move ScenarioPresetConflictError out of pyrit.exceptions into the storage
  module as a ValueError subclass and drop its inert status_code.
- Drop the misleading is_builtin guard in save_preset, which claimed to
  protect built-ins while acting on caller-supplied provenance.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Scenario defaults already cover most preset fields, so a built-in preset restating them duplicates shipped data. The only case built-ins uniquely serve is multiplicity, which no shipped preset needs yet, so the machinery is deferred to the PR that introduces the first real built-in preset.

- Remove ScenarioPresetProvenance, provenance, and is_builtin.
- Delete ScenarioPresetRegistry, which without built-ins was a pass-through to storage, and move its default-directory logic into ScenarioPresetStorage.
- Re-adding provenance later is not a migration: older documents lacking the key take the default.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@hannahwestra25 hannahwestra25 changed the title [DRAFT] FEAT: Add ScenarioPreset model, storage, and registry [DRAFT] FEAT: Add ScenarioPreset model and storage Oct 1, 2026
@hannahwestra25
hannahwestra25 marked this pull request as ready for review October 1, 2026 22:05
@hannahwestra25 hannahwestra25 changed the title [DRAFT] FEAT: Add ScenarioPreset model and storage FEAT: Add ScenarioPreset model and storage Oct 1, 2026
…ents

Listing no longer returns names that the single-document operations reject. FileDocumentStorage._list_documents now filters enumerated names through validate_registry_name and logs a warning for the ones it skips, so list_scripts() and list_presets() only return names that get/save/delete will accept.

This fixes a 500 from the custom initializer list endpoint whenever the script directory held an ordinary file like __init__.py or My-Script.py, and a misleading 400 'Invalid registry name' from the delete endpoint for a file that demonstrably exists.

Also: ScenarioPreset now forbids unknown fields so a misspelled key fails loudly instead of being silently dropped; the preset name is no longer duplicated inside the document, since the document name is authoritative; get_preset_version() lets a caller recover a preset name whose document is malformed; and RunScenarioRequest.max_dataset_size no longer describes the removed per-dataset behavior.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment thread pyrit/registry/file_document_storage.py Outdated
Comment thread pyrit/registry/file_document_storage.py Outdated
@richlundeen

Richard Lundeen (richlundeen) commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

My biggest hesitation is that it is difficult to validate the design without seeing the complete preset workflow working together.

I like the overall design, but I would prefer to review the persistence contract alongside the CRUD, launch integration, and UI that consume it. Otherwise, we risk committing to storage and versioning decisions before we can validate them through a real user flow.

Could we combine the first three planned PRs into one PR, or keep them as a stacked set that lands together? The individual diffs can remain small, but we would be able to review and merge them as one complete, usable feature.

I'm flexible here, but I think it's a good approach. WDYT?

Copilot AI added 3 commits October 2, 2026 13:04
Listing read and decoded every matching file before checking whether the
name was one it could address, so a single undecodable file raised out of
list_presets and list_scripts and hid every other stored document. Each
backend now validates the name first, reads per document, and skips one it
cannot read with a warning.

Decoding moves out of storage into the subclasses, which know how to report
a document they cannot interpret. That also makes the version token hash the
exact bytes on disk. Text mode had been rewriting line endings on Windows, so
the token described the in-memory string rather than the stored file, and a
raw-byte read for version checks alone would have made every update conflict.

Local writes stage the content in a sibling temporary file and move it into
place, so a failed write leaves the previous document intact instead of
truncating it, and a concurrent reader never sees a partial document.

Initializer source is normalized on decode. Callers materialize it back to a
file in text mode, which would otherwise translate a stored CRLF into CRCRLF.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Exposes the preset persistence layer over REST so a preset can be created,
read, edited, deleted, and turned into a run request.

Reads are open; mutations require an admin. Unlike custom initializers,
presets are data rather than executable code, so they are not gated behind
an allow_ switch.

POST creates a preset that must not already exist and PUT requires the
version returned when the preset was read, so a concurrent edit is reported
as a 409 instead of being silently overwritten.

References that do not resolve against the live registry are reported as
advisory issues on the response rather than rejected, keeping a preset
authored on one deployment storable on another.

POST /{name}/resolve merges a preset with the launch-owned fields it omits
and returns an ordinary RunScenarioRequest for the existing run endpoint.
Resolving server-side keeps one implementation of the merge, so the
distinction between unset and set-to-the-default cannot drift between
clients, and leaves the launch path itself untouched.

Storage is synchronous file or blob I/O, so every call into it runs on a
worker thread rather than blocking the event loop.

Adds scenario_presets_source so presets can be persisted to a shared Azure
Blob container instead of the default local directory.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Adds the frontend for scenario presets on top of the preset API:

- ScenarioPresetLibrary lists stored presets, surfaces validation
  issues from the backend, disables Launch for presets this
  deployment cannot run, and supports delete with confirmation.
- ScenarioPresetEditor creates and updates presets, pinning the
  scenario-owned fields only. Launch-time settings (target,
  concurrency, retries, labels) are deliberately absent.
- LaunchPresetDialog collects the launch-time settings, resolves the
  preset into a run request, and starts the run.

Extracts the scenario-owned configuration out of ScenarioDetail into
shared modules (scenarioConfigForm, ScenarioTechniqueSelector,
ScenarioDatasetFields, scenarioRunLimits) so the launch form and the
preset editor build the same config from one implementation.

Reached from the scenario catalog; routed under /scanner/presets. The
/edit suffix keeps /scanner/presets/new from shadowing a preset
literally named "new".

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@hannahwestra25 hannahwestra25 changed the title FEAT: Add ScenarioPreset model and storage FEAT: Add scenario presets - model, storage, API, and UI Oct 2, 2026
The modal covers the library card, so the operator previously committed to a
run knowing only the preset and scenario name. Restate the pinned techniques,
datasets, dataset cap, and baseline choice, rendering omitted fields as
scenario defaults to match server-side resolution.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@hannahwestra25 hannahwestra25 changed the title FEAT: Add scenario presets - model, storage, API, and UI FEAT: Add ScenarioPreset Oct 2, 2026
@richlundeen

Copy link
Copy Markdown
Contributor

Can we separate persistence from this PR and address it as a shared framework capability? Scenario presets need it, but so do targets and other named registry configurations. I would prefer one design for what gets persisted, how references and credentials are handled, and how updates are versioned, rather than establishing a separate storage path for each component.

For this PR, could we keep the ScenarioPreset model and non-persistent runtime use—for example, presets supplied through code or initializers—and defer durable storage and the API/UI behavior that depends on it? That lets us settle the preset contract without also committing to a persistence architecture.

@hannahwestra25

Copy link
Copy Markdown
Contributor Author

Can we separate persistence from this PR and address it as a shared framework capability? Scenario presets need it, but so do targets and other named registry configurations. I would prefer one design for what gets persisted, how references and credentials are handled, and how updates are versioned, rather than establishing a separate storage path for each component.

For this PR, could we keep the ScenarioPreset model and non-persistent runtime use—for example, presets supplied through code or initializers—and defer durable storage and the API/UI behavior that depends on

hmm agreed that targets and other instance-backed configs need a persistence design, and this PR shouldn't try to settle it. But I was treating presets as closer to initializers and the environment/config files, and followed that pattern deliberately: a preset is a configuration, not an object in its own right the way a target or converter is.

That split seems to be where the existing persistence boundary already falls. .pyrit_conf, .env files, and custom initializer scripts all persist today, each a named document carrying its own copy of the same sha256-version + expected_version + local-or-blob logic. Custom initializers are the closest analogue — created in the UI, written to disk or blob, admin-gated, reloaded at boot. Presets do the same thing with a strictly smaller blast radius: inert JSON validated against a model with extra="forbid", versus arbitrary Python that gets imported and executed at startup.

Targets and converters live in DefaultInstanceRegistry — in-memory, lost on restart. Persisting those means storing a construction recipe plus credentials, which is genuinely unsolved. Presets are target-agnostic, so they hold no secrets and contribute nothing to that design either way.

The resolving registry references that can go stale when a scenario or technique is renamed is a new concept. but that's more of a side effect of the storage decision

@richlundeen

Copy link
Copy Markdown
Contributor

After considering the configuration-versus-runtime-instance distinction, I would revise my earlier suggestion to postpone preset persistence entirely.

General recommendation

Consolidate the document-storage mechanics, not the runtime objects. Preset documents, initializer scripts, configuration files, and future target construction recipes can share storage operations without sharing their models or loading behavior.

The shared layer should own local/Blob I/O, missing-document handling, opaque versions, and conditional updates with explicit concurrency guarantees. Typed adapters should own serialization and validation. Reference resolution, component construction, credentials, and API authorization should remain outside that layer. Environment-file protections and read-only source rules must remain intact when those services adopt shared storage.

An instance registry addresses named runtime registration and lookup; it does not itself solve persistence. I would not require presets to implement a component identity contract or introduce a second in-memory copy just to reuse an instance registry.

Recommendation for this PR

Keep preset persistence rather than make the feature live-instance-only. The existing FileDocumentStorage extraction, shared with custom initializers, is a useful starting point.

  • Keep ScenarioPresetStorage as a thin preset-specific serialization/validation adapter.
  • Put reusable version/conflict mechanics in the shared storage layer. Enforce the update guarantee: checking a hash and then writing unconditionally can still lose concurrent edits; atomic replacement only prevents partial documents.
  • Defer migration of the other configuration services, target construction-recipe persistence, credential resolution, and broader registry changes to separate work.

The scope should be reliable configuration-document persistence now, with broader component persistence later. It should not require solving target reconstruction before presets can be saved.

Copilot AI added 6 commits October 7, 2026 10:29
Preset storage had its own sidecar-lock implementation alongside the generic document store's. Consolidating on a single conditional-write path keeps the two from drifting and leaves scenario_preset_storage.py a thin adapter.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Every scenario-owned preset field is tri-state, where an omitted field keeps tracking the scenario default. The editor has to prefill those fields to render a control for them, so it was writing all of them back on any save and freezing this deployment's defaults into the document.

The editor now emits a field only when the operator moved it off the scenario default or the stored document already pinned it. Alongside that:

- Aggregate techniques are part of the server's allow-list, so the editor no longer reports them unavailable nor replaces them with the default set on save.
- A tag toggle derives from the current selection rather than the rendered checkboxes, so a pinned aggregate survives it.
- Stored scenario_params the editor renders no control for are carried through and named in an info bar instead of being dropped.
- A baseline pin the forbidden policy hides from the form is preserved rather than silently flipped.
- list_presets_async gathers its per-preset lookups, matching scenario_service.
- configure_source runs off the event loop, since it may create the default preset directory.
- scenario_presets_source is documented in .pyrit_conf_example.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Most of this is ordinary: the preset and run-request models grew fields on both
sides, and both sides added a route module. RunScenarioRequest keeps main's
request-limit types while retaining this branch's max_dataset_size description,
which deliberately matches ScenarioRunSizeEstimateRequest.

One conflict git could not see. Upstream replaced the dataset-sizing heuristic
that summed the seed groups a deployment happened to have loaded with a limit
the scenario declares for itself, and deleted the old code. This branch had
already copied that heuristic verbatim into a new shared module, so the delete
applied cleanly and the copy survived, which would have reinstated a number that
means something different on every install. The shared helper now reads the
declared limit, and a scenario that sizes itself by prompt generation reports no
default and disables the field, so a preset cannot pin a cap the scenario will
never apply. A preset authored through the API that already pins one is still
shown and re-emitted, matching how a forbidden baseline is preserved.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A preset is launched by copying its fields into RunScenarioRequest, which caps list and identifier sizes. The stored model declared none, so an oversized preset saved cleanly and only failed later at launch, when the operator is furthest from the field that caused it. Reuse the same annotated types so it fails at save.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The selector appends in click order while a scenario lists its defaults in
registry order, so unchecking and rechecking a default technique produced a
reordered-but-identical list. The structural compare read that as an edit and
pinned the whole default set, costing the preset the tracking it exists to keep,
and the sticky rule then re-pinned it on every later save. The backend resolves
techniques through a set, so the order never carried meaning.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The age check and the unlink were separated, so two writers seeing the same
stale lock both removed it: the first went on to take a fresh lock and the
second deleted that one, leaving both inside the read-compare-replace with the
same expected version. Both would then match and replace, losing the first
writer's content while returning it success and a version token describing
bytes that were never stored.

Breaking a lock now happens under a second exclusive file, with the staleness
re-checked while it is held so a lock released and retaken in between is left
alone. A breaker older than the stale window can only be an orphan, since
holding it spans a stat and an unlink, so it is discarded rather than wedging
reclamation shut.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
<div className={styles.header}>
<div className={styles.headerText}>
<Text id="scenario-preset-library-title" as="h1" size={600} weight="semibold">
Scenario presets

@richlundeen Richard Lundeen (richlundeen) Oct 7, 2026 •

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.

Please move Scenario presets into Registry, as a third tab next to Targets and Converters, not under Scanner. A preset is a saved, named definition, which is what the Registry manages.

Suggested changes:

  • RegistryLayout.tsx: add <Tab value="scenario-presets">Scenario presets</Tab> and select it from the path.

  • App.tsx / presetRoutes.ts: move the routes to /registry/scenario-presets, /registry/scenario-presets/new, and /registry/scenario-presets/:name/edit. Keep a redirect from /scanner/presets.

  • Remove the Scanner Presets button and the "Back to the scanner catalog" link.

  • Tab header text: "Reusable scenario configurations. A preset pins what to test; you choose the target at launch." Put New preset in the header, the same as the other Registry tabs.

  • Use a table, not cards, so this tab looks like Targets and Converters. Columns:

    1. Name / purpose (description)
    2. Scenario
    3. Run-size estimate
    4. Author

    Row actions: Launch, Edit, Delete. Launch opens the existing launch dialog, and the run then shows in the normal run view.

Two backend changes are necessary for these columns:

  • Author: ScenarioPreset has no author field today. Please add an optional author. The backend can set it from the signed-in user at create time.
  • Run-size estimate: ScenarioPresetResponse has no estimate, and the list endpoint omits it. Please return an estimate for the preset's own techniques, datasets, and limits, not the scenario default, so the table does not need one request for each row.

// Derived from the current selection rather than rebuilt from the rendered options, so a
// selection with no checkbox of its own — a preset pinning an aggregate technique such as
// `all` — survives a tag toggle instead of being silently dropped.
const retained = selectedTechniques.filter((name) => shouldSelect || !memberNames.has(name))

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.

P1 — Aggregate selections are hidden and cannot be narrowed correctly.

For techniques: ["all"], every concrete checkbox appears unchecked. Selecting only the multi-turn tag produces ["all", "crescendo"], so the backend still runs all techniques. Clearing the tag leaves "all" intact. The new component test explicitly expects this behavior.

Preserving the aggregate fixes the earlier data-loss problem, but leaves a mismatch between the controls and execution. Please display aggregates as selectable, removable controls, or expand their effective selection and replace the aggregate when the user edits it. An unrelated description edit should still preserve the original selector.

return []

known = set(scenario.all_techniques) | set(scenario.aggregate_techniques)
unknown = [technique for technique in preset.techniques if technique not in known]

@richlundeen Richard Lundeen (richlundeen) Oct 7, 2026 •

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.

P2 — Valid converter-qualified presets still cannot launch through the library.

A valid token such as role_play:converter.translation_spanish is marked unknown, even when its technique and converter are registered. That issue disables Launch. The editor also classifies the token as unavailable and can remove it on save.

Both surfaces should use the launch rules, including modifier validation, not separate bare-name checks.

Where the shared rules should live. Today, only ScenarioConfigurationResolver.resolve_techniques_and_converters in pyrit/backend/services/scenario_configuration_resolver.py parses the token grammar. It converts technique:converter.<name>:... to a technique enum member plus converter instances from ConverterRegistry. Scenario itself accepts only the resolved scenario_techniques and technique_converters. Because the parser is in the backend, the preset checks, a future preset registry, and the CLI cannot reuse it.

Suggestion: move the parsing and resolution to pyrit/scenario/core/technique_tokens.py, next to _technique_resolution.py. Input: tokens + the scenario's technique class. Output: technique members + converters. The token grammar belongs to scenarios, and scenario core already looks up registries. Do not put it in pyrit/models, because models should not look up registries. Then the backend resolver and the preset checks call the same function, and this mismatch cannot recur.

setSubmitting(true)
setError(null)
try {
const request = await scenarioPresetsApi.resolve(preset.name, {

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.

P2 — Launch can still execute configuration different from the confirmation dialog.

If another deployment edits the preset after the library loads, the dialog shows the old configuration while /resolve returns the new one. This can change datasets, limits, or even the scenario.

Referencing the latest preset by name is reasonable for a reusable definition. It should not mean silently changing what the user just confirmed. Please pass the displayed version to resolution, and require confirmation after a mismatch.

if held_seconds is None:
return False
try:
lock_path.unlink()

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.

P2 — Local lock recovery assumes age proves that the writer died.

Any lock older than 60 seconds is removed without checking its owner. A process paused after the version check, or delayed by filesystem I/O, can still be alive. A second writer can remove its lock and save; the first can then resume and overwrite that edit. The first writer's cleanup can also remove the replacement lock.

The breaker file serializes recovery attempts, but does not establish that the original owner has stopped. Please use an OS-backed advisory lock released when the process exits, or another ownership-safe mechanism. Increasing the timeout does not fix the correctness issue.

Args:
source (str | None): The configured source, or None to use the default directory.
"""
self._storage = ScenarioPresetStorage(source=source)

@richlundeen Richard Lundeen (richlundeen) Oct 7, 2026 •

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.

Recommendation: make presets a Registry, the same way AttackTechniqueRegistry holds technique factories.

AttackTechniqueRegistry is a Registry whose instances hold AttackTechniqueFactory definitions, not live attacks. A preset is the same kind of object: a named, reusable definition. Suggested shape:

  1. pyrit/registry/components/scenario_preset_registry.py: ScenarioPresetRegistry(Registry["ScenarioPreset", ScenarioPresetMetadata]).

    • _discover registers no classes, the same as for techniques.
    • self.instances holds ScenarioPreset definitions. ScenarioPreset implements Identifiable.
    • The registry owns ScenarioPresetStorage, configured from scenario_presets_source. At startup, it loads stored presets into instances, like InitializerRegistry.register_stored_initializers. Save and delete write to storage first, then update instances. Keep the storage version with each entry.
    • Code and initializers can also register presets with no storage.
    • The copy in memory can fall behind edits from another deployment. That is acceptable: the version check at save and at launch catches it.
  2. One validation path. check_preset must use the launch rules, not a second set of name checks:

    • scenario and parameter declarations from ScenarioRegistry metadata
    • the shared technique-token resolver (see below), which handles technique:converter.x modifiers and aggregates such as all
    • resolve_declared_params for value and type checks

    Then _unknown_technique_issues and _unknown_parameter_issues can go away. Keep _forbidden_baseline_issues only if launch does not already reject that case.

    Move the token resolver. Today it is ScenarioConfigurationResolver.resolve_techniques_and_converters in pyrit/backend/services/, and pyrit/registry cannot import backend code. Move the parsing and resolution to pyrit/scenario/core/technique_tokens.py, next to _technique_resolution.py: tokens + the scenario's technique class → technique members + converters from ConverterRegistry. The backend resolver, the preset registry, and later the CLI then use one implementation of the grammar.

  3. Resolve against a version. resolve(...) takes the displayed expected_version and returns 409 on a mismatch, as save already does. This fixes the confirm-versus-launch drift.

  4. Thin service. ScenarioPresetService only maps registry results to API models, the same as TargetService / ConverterService. ResolveScenarioPresetRequest reuses the RunScenarioRequest run fields and bounds.

  5. Frontend. Show it as a Registry tab (see the UI comment on ScenarioPresetLibrary.tsx), backed by the same list and metadata calls.

Later, saved target and converter definitions can use the same pattern. This PR does not need to build that part.

description: str | None = Field(None, description="Human-readable summary of what this preset tests")
techniques: _RequestTechniques | None = Field(None, description="Technique names; None uses the scenario default")
dataset_names: _RequestNames | None = Field(None, description="Dataset names; None uses the scenario default")
max_dataset_size: int | None = Field(None, ge=1, description="Maximum selected logical seed groups")

@richlundeen Richard Lundeen (richlundeen) Oct 7, 2026 •

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.

Merge order: I prefer that #2956 merges first.

Please link this PR to #2956 (Make scenario dataset sources and limits explicit). It keeps the API name max_dataset_size, but it changes the accepted values:

Value Behavior in #2956
Omitted, null, "", or "default" Use the scenario default
Positive integer Set the total limit on selected groups
"all" Remove the total limit (per-dataset limits still apply)

Internally, #2956 adds explicit DatasetSource configuration, with separate per-dataset and total limits. with_overrides() keeps the source settings when it applies overrides.

This is not a hard block. #2956 keeps the meaning of an integer and of null, so preset documents saved with this PR stay valid. The gap is that this preset accepts only int | None, and the shared form parser cannot represent "all".

When both PRs are merged, update these parts to use the same contract and normalize_dataset_limit:

  • the preset model
  • the shared controls
  • serialization
  • resolution
  • examples

If #2956 merges first, do this in this PR. If this PR merges first, do it in a follow-up PR.

Suggested dependency note for the PR description:

Related to #2956. Prefer to merge #2956 first. Then update presets to use the same dataset-limit normalization and GUI controls, including "all".

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.

4 participants