Skip to content

FEAT: Delete saved registry entries (part 20) - #4071

Open
varunj-msft wants to merge 238 commits into
microsoft:mainfrom
varunj-msft:varunj-msft/13178-Delete-Saved-Entries
Open

varunj-msft wants to merge 238 commits into
microsoft:mainfrom
varunj-msft:varunj-msft/13178-Delete-Saved-Entries

Conversation

@varunj-msft

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

Copy link
Copy Markdown
Contributor

Description

Part 20 of saving Registry entries across restarts. Stacked on part 15 (#4067), with part 5 (#3965) and Hannah's #3656 merged in first; part 21 (#4072) builds on it. This PR's own changes: varunj-msft/PyRIT@0499601...varunj-msft/13178-Delete-Saved-Entries

Design: 4.7 Editing and deleting, 4.6 When a saved entry can't be restored. Where every part fits: PR map.

This is one of the changes for saving Registry entries across restarts. It needs #3965 (the list of entries that couldn't be restored, and Retry) and #3656 (version-checked deletes). Until the earlier PRs merge, the diff here also shows their changes.

A saved target, converter, scorer or technique couldn't be deleted from the app. The only way was to delete its file under registry_recipes_source and restart, even for an entry that failed to restore and kept the backend from being ready.

  • GET /api/saved-entries/{kind}/{name} describes a saved file: its version, why it can't be used if it can't, who saved it, and whether you may change it. DELETE /api/saved-entries/{kind}/{name}?expected_version=<version> deletes the file and removes the entry it restored. kind is targets, converters, scorers or techniques.
  • These are management routes, so they answer while the backend isn't ready. An entry that failed to restore can be deleted, and Retry then finishes recovering. A separate resource also stays clear of DELETE /api/targets/{name} in FEAT: Support personal tokens with per-caller target credentials #4024, which removes Temporary targets under its own rules.
  • Who may delete an entry comes from its file: the person who saved it or an administrator, or anyone when sign-in is off (403 otherwise). A file that doesn't say who saved it needs an administrator.
  • A delete sends the version it read (422 without one, 409 if the file changed). A file that can't be read is deleted without one.
  • A delete is refused (409, naming them) while another saved entry names the entry, or a registered entry of any origin holds it, including through a technique's converters. It waits up to two seconds for requests in flight to finish, then refuses while a run, send or estimate could be using the entry.
  • The running entry is removed before its file is deleted and put back if the delete fails. Its resources are released unless another name still holds the same object. A startup entry with the same name stays.
  • Saving and deleting take turns, and a save checks again that the entries it names still come back after a restart, so a delete can't miss a save that names the entry.
  • DELETE /api/converters/{id} still refuses a saved converter (409), and its message points to the new route.
  • The Targets, Converters, Scorers and Techniques tables have Delete on saved rows, and the banner for entries that couldn't be restored has Delete on each one. The dialog says who saved the entry and whether you may delete it, and offers Look again when the file changed since it was read.
  • The runtime banner now renders inside the app's theme, so it and the dialog it opens are drawn with the theme's colors. It rendered outside the theme before.
  • Merging FEAT: Recover from a failed restore of saved registry entries without a restart (part 5) #3965 and FEAT: Optional version check when deleting scenario presets and suites #3656 here needed two small fits: the explanation shown for a credential refusal now also covers a key or headers the entry would take from this server, and the e2e preset storage guard passes the new version check through.

What stays the same: saving and restoring work as before, except that a save waits for a delete in progress and checks again that what it names still comes back. DELETE /api/converters/{id} still removes Temporary and startup converters from the live registry. Startup entries have no saved file, so they can't be deleted here (404).

Screenshots: Delete on a saved target in the Targets table, the dialog, and the restore banner with Delete on each entry, including the dialog for an entry someone else saved.
delete_1_saved_target_row_with_delete
delete_2_delete_dialog
delete_3_restore_banner_with_delete
delete_4_delete_dialog_not_yours

Not covered:

varunj-msft and others added 30 commits October 26, 2026 20:05
…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.
DELETE /api/scenario-presets/{name} and /api/scenario-suites/{name} accept an
optional ?expected_version= query. When supplied, the delete is refused with 409
if the stored document changed after the caller read it, so confirming a delete
in the GUI cannot discard an edit the caller never saw. Without it the delete
stays unconditional, preserving existing callers.

Locally the version is compared inside the existing per-document lock; on Azure
Blob the delete carries the read ETag with MatchConditions.IfNotModified. A
versioned delete of a document that is already gone still reports 404.

The preset and suite libraries send the version they loaded and, on 409, close
the confirmation, reload, and explain that the item changed.

Follow-up to review feedback on microsoft#3057.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
# Conflicts:
#	tests/unit/backend/test_converter_service.py
The OpenAI SDK adds that variable's headers to every request, so a saved
OpenAI target would send whatever the server puts there, which can be a
credential. While it's set, saving an OpenAI target is refused and a
saved one isn't restored; the message names the variable, never its
value. Targets declare such variables in server_header_variables.
Saving converters and scorers follows the same rules as saving targets now: a type that only a scan's launch script added can't be saved, a class without cached metadata is checked the way restore checks it, only a taken name is reported as a conflict, a storage failure logs its short reason, and the owner is recorded only when the entry is saved.
Saving a technique follows the same rules as the other saved entries now: an attack type that only a scan's launch script added can't be saved, a name or tag clash while registering is a conflict, and the owner is recorded only when the technique is saved. A held technique name is refused when a technique is checked before saving too.

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