Repository navigation
FEAT: Read saved registry recipes (part 2) - #3892
varunj-msft wants to merge 44 commits into
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues require changes to the registry recipe models or storage reader.
No open issues 🎉
What changed in this PR
Adds the models and storage reader needed to preserve registry entries across restarts. This second stacked PR builds on #3891’s document-listing support; entry rebuilding and startup restoration remain for later PRs.
Changes:
- Defines versioned component and technique recipes, plus restore-report types with JSON-safe text.
- Reads recipes from per-registry local or Azure Blob folders, preserving SAS queries and reporting invalid, unreadable, newer-format, and ignored files.
- Adds unit coverage for parsing, versions, validation, storage failures, and report serialization without changing existing callers.
| File | Description |
|---|---|
tests/unit/registry/test_registry_recipe_storage.py |
Tests recipe loading, validation, versions, Blob paths, and report serialization. |
tests/unit/registry/test_file_document_storage.py |
Tests issue-aware local and Blob listings and safe error messages. |
pyrit/registry/registry_recipe_storage.py |
Adds per-registry storage, recipe parsing, and failure reporting. |
pyrit/registry/file_document_storage.py |
Adds issue-aware listings while retaining legacy listing behavior. |
pyrit/models/request_limits.py |
Adds the shared technique-description text limit. |
pyrit/models/catalog/registry_recipe.py |
Defines recipe schemas, references, and restore-report models. |
pyrit/models/catalog/__init__.py |
Exposes the new models through lazy exports. |
Review effort: Balanced
…s that can't be checked
…navailable storage
Richard Lundeen (richlundeen)
left a comment
There was a problem hiding this comment.
Requesting changes. My review on #3908 has the main comment about the direction for the restore work; this PR stays as it is apart from the inline comments below. Inline comments marked Blocking need to be addressed before merge; Non-blocking ones are judgment calls.
FileDocumentStorage now raises DocumentStorageUnavailableError when the folder or container can't be used, and DocumentUnreadableError when one document exists but can't be read. The message names the location, with any SAS token hidden, and a short reason. One backend handler maps both to 503 and logs the location and reason, with the traceback at debug level, so the per-route storage helpers are gone. A single read, a conditional save and a delete now follow the listing's rules: a folder, pipe or broken link named like a document is unreadable rather than missing, and on Blob only a deleted blob counts as missing. Blob sign-in, permission, network and missing-container errors stop the listing instead of marking every file unreadable. Each unreadable report keeps the real file name and the original error, and the listing's documents are read-only. Initializer delete removes a script that can't be read or decoded, and registering one no longer turns a storage write failure into a 400. Tests patch module-level aliases instead of os.
List recipes escapes the OS text it reports, such as a file name that isn't UTF-8, and a recipe whose JSON holds an unpaired surrogate escape is reported as invalid, so the models no longer need a JSON-only serializer. Recipes may start with a byte-order mark. A document without schema_version reads as version 1, and each file is checked against the format version of the model that reads it. Validation summaries shorten each location part and the whole message, since a location can repeat a stored key. Recipes and StoredRecipe are no longer frozen, since their params and tags never were, and StoredRecipe checks that its recipe type matches its registry. The listing's recipes are read-only, and it keeps the error behind each recipe that couldn't be read, for the log.
… text Recipes now share one format version, RECIPE_SCHEMA_VERSION, and a file without one still reads as version 1. A saved target keeps the auth mode its create request chose as its own field, so restore can replay that request; only a target may have one. Every failure message is escaped and shortened where it's built, so a stored key with a new line or a terminal escape, or a long version number, can't change a log or run on. A params value nested too deeply for pydantic is reported as nested too deeply, and the technique recipe says why it copies the create request's fields.
Richard Lundeen (richlundeen)
left a comment
There was a problem hiding this comment.
Automated review completed. No blocking concerns remain. This review records source coverage; merging is handled separately.
# Conflicts: # pyrit/backend/middleware/error_handlers.py # pyrit/backend/routes/scenario_presets.py # pyrit/backend/routes/scenario_suites.py # pyrit/registry/file_document_storage.py
…y the other storage tests do
Its text can carry a SAS URL. The 503 handler no longer logs it as a debug traceback, and the scenario suite service no longer attaches it to its warning; both log the storage message, which has no credentials.
Description
This is one of the changes for saving Registry entries across restarts. It builds on #3891. Until #3891 merges, the diff here also shows its changes.
Problem. There's no file format or storage for a saved target, converter, scorer or technique.
Change.
pyrit/models/catalog/registry_recipe.py: the recipe models, exported frompyrit.models.catalog.schema_version,class_module,class_nameandparams. A target recipe also holdsauth_modenext toparams, as the target create request does; any other kind with anauth_mode, or a recipe filed under the wrong kind, is rejected.RECIPE_SCHEMA_VERSIONsets the format version for both models and the storage check.credentialsis reported, not silently read.pyrit/models/request_limits.py: addsMAX_TEXT_LENGTHfor the technique description. FIX: Apply the request limits to request fields added since the last limits PR #3595 adds the same line, so the two merge cleanly; until it lands,backend/models/common.pykeeps its own copy of the same value.pyrit/registry/registry_recipe_storage.py:RegistryRecipeStorage, aFileDocumentStoragewith one folder per registry (targets/,converters/,scorers/,techniques/) under one source.list_recipes()uses the listing from the previous PR. It returns the parsed recipes with their versions (the SHA-256 of the bytes), a failure for each unreadable, invalid or newer-format file, and the ignored files."\n\x1b[31mFAKE"or a 4,000-digit version can't break a log line. A recipe with text that isn't valid UTF-8 anywhere in it is rejected as invalid, and file names with invalid bytes are escaped in backslash form, so the report can always be sent as JSON. Files are read asutf-8-sig, and a document nested too deeply is reported as such.What stays the same. Nothing calls this yet; #3908 (part 3) rebuilds entries from these recipes. No API, config or behavior change.
Tests and Documentation
tests/unit/registry/test_registry_recipe_storage.py: recipes and their versions, field limits and unknown fields, failures for unreadable, invalid and newer-format files, ignored names, Blob folders that keep a SAS token in the query, and report text that isn't valid UTF-8 (a recipe holding it is invalid, and a file name with invalid bytes is escaped).paramsnested 300 levels deep.pytest tests/unit/registry tests/unit/models -n 6: 3,979 passed, 5 skipped, 4 xfailed