Repository navigation
FEAT: Save API-created targets, converters, and scorers across restarts - #3058
Draft
varunj-msft wants to merge 8 commits into
Draft
varunj-msft wants to merge 8 commits into
varunj-msft wants to merge 8 commits into
Conversation
Move local-directory and Azure Blob document I/O into a shared base so other configuration documents can reuse it. CustomInitializerStorage keeps its public API. Documents are read as bytes, and local writes replace the file atomically. A conditional write refuses to replace a document whose stored bytes no longer match the version the caller read: Blob creates use overwrite=False and replacements send the ETag of the bytes just compared, and local writes hold an operating-system lock on a file beside the document across the read, compare, and replace, which the system releases if the writer exits. Behavior change: document names are validated before they become paths, so a name such as "../x" can no longer address a file outside the source. Listing skips files whose names it cannot address, such as "My-Script.py", with a warning; before, strict startup failed on them. Co-authored-by: hannahwestra25 <hannahwestra@microsoft.com>
A delete that checks a version and then removes the document unconditionally can discard a concurrent edit, so the comparison and the removal now happen together, as a conditional write's do: a Blob deletion sends the ETag of the bytes just compared, and a local one holds the document's lock across the read, compare, and unlink. A version is the SHA-256 of the stored bytes, so a hand edit is also a conflict. Tests cover conditional writes and deletes on both backends and the local lock: a held lock times out, a lock file left by a writer that exited does not block the next writer, and concurrent replacements from one version let exactly one writer through.
An instance recipe is the type, constructor arguments, and environment-variable credential references needed to rebuild a named target, converter, or scorer. Recipes are JSON documents in a local directory or Blob container configured by instance_recipes_source (default ~/.pyrit/instance_recipes). Each is stored under a name derived from its kind, a readable slug, and a digest of the exact instance name, so names with upper case, dots, or dashes and names that differ only in case keep separate documents. A document that cannot be parsed, or that the store fails to return, is reported with the reason instead of being skipped; one that does not say which instance it belongs to can be found under its own document name, and a name of that shape always means the document itself. A replaced recipe can be put back byte for byte. FileDocumentStorage can now also return the read error of each document it could not read; its listing still skips them with a warning. A local directory that cannot be listed now fails the listing, as a failed Blob listing does, instead of reading as empty; stored custom initializers get the same behavior. ComponentIdentifier.get_sensitive_parameter_names also lists the parameters that can carry a credential (headers, http_request, cookie, and azure_speech_key), so Parameter.sensitive marks every constructor parameter whose value is a credential or can carry one. A test fails when a new parameter whose name looks like a credential is not classified. CredentialNames recognizes the names that label a credential (a settings field, header, or query parameter), compared by their letters and digits in any letter case, and UrlCredentials the credentials a URL carries, which it can also mask in text as written, so a URL its parser rejects is masked too; a switch value such as true or 0 is never one. It reads user information as a URL parser does, except that when what follows it cannot be a host and a port, a password's unencoded "/", "?", or "#" is read through to the last "@"; the credentials of a URL written in another URL's query or fragment, such as a proxy's upstream URL, count too, each parameter read on its own. The storage uses them to keep the source's SAS signature out of error messages.
References resolve by name when an instance is built, so a saved recipe must be rebuilt after the saved recipes it names. The planner orders recipes targets, then converters, then scorers, each by name, and reports recipes that cannot be rebuilt: a name that is already registered, an unknown type, a reference to a missing instance, members of a reference cycle, and recipes that depend on any of those. Recipes blocked only by a reference are listed with everything they reference, so their reason can be revisited later.
An HTTPTarget identifies itself by the URL of its request template. When that URL carries a credential, such as an Azure Functions code in its query or a token as its user name, the credential became part of the identifier, which the API returns and memory stores. The identifier now shows the URL with those values replaced by ***. A credential must be percent-encoded, as URLs require, to be recognized whole: an unencoded "/", "?", or "#" can end a user name or password early. Requests still use the template unchanged. Identifiers of HTTPTargets whose request URL carries a credential change.
…ross restarts
Targets, converters, and scorers created through the API now survive a
backend restart and a live reinitialization. Each create, replace, and
delete writes through to a saved recipe in the configured
instance_recipes_source (a local directory by default, or an Azure Blob
container), and the backend rebuilds the saved instances after the
initializers run.
- Saving is the commit point: an instance is built and mapped off the
registry, its recipe is written with a conditional write, and only then
is it registered. A failed registration removes the recipe again, or
puts the replaced recipe back byte for byte. The instance is built from
a copy of the recipe, so a constructor that changes its arguments cannot
change what is saved.
- Credential parameters, the ones the type metadata marks sensitive, are
not saved as values. One is sent as
credentials.<name> = {"env_var": "<VARIABLE>"}. Sending it as a value is
rejected, and so is a value that carries a credential in a way the checks
recognize: a URL with a user name, password, SAS signature, or key in its
query (a URL written in another URL's query counts too), or a credential field
inside free-form settings such as httpx_client_kwargs. Credential
references require administrator access,
identity authentication rejects a reference or value that the type
metadata marks as replacing it (headers and sas_token), and errors from
building an instance have the referenced values, and the secrets inside
them, removed (one shorter than four characters where it stands alone as
a word), and the credentials of any URL they quote masked.
The constructor parameters a target's authentication mode implies, such
as AzureBlobStorageTarget's auth_mode, are applied at every build,
restores included, rather than saved.
- PUT replaces a saved instance and DELETE removes it. Both require the
version returned when the instance was read, so a stale change gets 409
and a delete without a version gets 428. A saved instance that other
saved instances reference cannot be deleted, or changed while it is
restored, and a restored instance counts by what it was built from even
after its saved document changes outside the API. With the document's
current version, a replace applies to it while this PyRIT can still use
the document, and a delete by its name (or by its document name when
this PyRIT cannot use the document) removes it; deleting one whose
document no longer exists is refused until a restart, and only an
instance not built from a saved recipe is taken for an initializer's. A
saved instance whose type is no longer registered counts as referencing
every name its parameter values hold. While a
saved document cannot be read from storage, such a delete or change is
refused (503), because it may reference the instance. A saved document
that cannot be parsed can be deleted but not replaced, and names shaped
like a document name are reserved.
- Restore rebuilds instances after the ones they reference. An instance
that cannot be rebuilt (unset environment variable, unknown type, name
already taken by an initializer, unreadable document) is listed with the
reason in the list responses, and GET returns that reason, instead of
the instance silently disappearing. A restore that fails as a whole is
reported as restore_error, and while the store still cannot be listed,
creates and replaces are refused (503) because a restart could not
rebuild them. When a change makes an instance's references available,
its reason says a restart or reinitialization rebuilds it; a saved
instance that could not be restored, such as one whose name an
initializer took, does not make a reference available.
- A change that has committed still succeeds if freeing what the replaced
instance owned fails; the failure is logged.
- Scorer names cannot be "types", which the types route would shadow.
Breaking: a create request that sends a credential parameter as a value,
or a value that carries a credential, now gets 400 (with identity
authentication an api_key value is still dropped, as before), and deleting
a saved instance requires the version it was read at.
…n the GUI The GUI follows the saved-instance API: targets and converters created in it now survive a restart, and nothing it sends is a credential. - The target dialog asks for a name, since the target is saved under it, and for the name of a server environment variable instead of a value for the API key and every other parameter the type metadata marks sensitive, such as sas_token or an HTTPTarget's http_request. Identity authentication still clears the ones that would replace it. Only variable names are sent. - The converter dialog does the same for credential parameters, such as azure_speech_key, that the type metadata marks sensitive. - Saved targets get a Delete action, and removing a saved converter sends the version it was read at. - The targets and converters pages list saved instances that could not be restored, with the reason and a Delete action, and report a saved instance store that could not be read. - Playwright suites stop posting raw keys, name the targets they create, and delete saved converters with their version. The end-to-end backend saves its instances under the ignored dbdata/ directory, not ~/.pyrit.
Describe instance_recipes_source, the saved-instance REST contract (credential references and the values that are rejected, versions, PUT and DELETE, unrestorable instances), the GUI's name and environment variable fields, that live reinitialization now rebuilds instances created through the GUI or API, and that local end-to-end runs save what they create in the developer's store.
varunj-msft
marked this pull request as draft
October 9, 2026 19:55
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
Targets, converters, and scorers created through the API (and the GUI) now survive a backend restart and a live reinitialization, including local development with no Azure. Each create, replace, and delete writes through to a saved recipe (type, constructor parameters, references to other instances, and the names of the environment variables that hold its credentials), and the backend rebuilds the saved instances after the initializers run, in dependency order.
This follows the direction from #2515 and #2931: one shared document-storage layer, recipe separate from credential, no run-data database dependency.
Storage.
FileDocumentStorageis carried byte-for-byte from #2931's head8959c7b2d(commit 1, co-authored), conditional writes included: a version is the SHA-256 of the stored bytes; Blob usesoverwrite=Falsefor create and the ETag of the compared bytes for replace; a local write holds an operating-system lock on a file beside the document across read/compare/replace, which the system releases if the writer exits. Commit 2 adds the conditional delete recipes need on the same primitives, so this PR adds no dependency. Recipes live ininstance_recipes_source(default~/.pyrit/instance_recipes, or an Azure Blob container), independent ofmemory_db_type. A local directory that cannot be listed is a store error, as a failed Blob listing is, rather than an empty store (this also applies to stored custom initializers, whichglobused to read as none).Credential model (the build gate). Credential parameters (
api_key,headers,http_request,sas_token, and the rest ofComponentIdentifier.get_sensitive_parameter_names()) are never saved as values: a request names a server environment variable instead,"credentials": {"api_key": {"env_var": "OPENAI_CHAT_KEY"}}, which requires administrator access. Values that carry a credential in a recognized way are rejected: a URL with user information or a credential query parameter (a URL written in another URL's query or fragment, such as a proxy's upstream URL, counts too), a credential field inside free-form settings such ashttpx_client_kwargs, and akey/code/sigin query parameters, headers, or authentication settings in any shape (map, list of pairs, query string). OneCredentialNamesrule set serves the URL and field checks; names are compared by their letters and digits in any letter case, a number counts like text, and a switch value such astrueor0never counts. These checks are best effort and say so; a token in a URL path is not detected, and a credential written into a URL must be percent-encoded, as URLs require, to be recognized whole (user information is read as a URL parser reads it, except that when what follows it cannot be a host and a port, a password's unencoded/,?, or#is read through to the last@; in a URL written inside another URL's query, an unencoded&or#leaves the credential unrecognized). Errors from building an instance have resolved credential values, and the secrets inside them, removed (one shorter than four characters where it stands alone as a word; every value of a credential that is a JSON object, such as aheadersmap, counts, so an ordinary one such asapplication/jsonis removed too), and the user information and credential parameters of any URL they quote are masked, read as written so a malformed request template's URL is covered. Type metadata marks credential parameterssensitive(each registry derives the flags from its identifier type); the server enforces those flags, and the target and converter dialogs ask for an environment variable for them.Conflicts, atomicity, failures. Saving is the commit point (build → save with CAS → register); the instance is built from a copy of the recipe, so a constructor that changes its arguments cannot change what is saved, and a failed registration removes the recipe or puts the replaced one back byte for byte.
PUTandDELETErequire the version that was read (409 when stale; aDELETEwithout one gets 428). A saved instance other saved instances reference cannot be deleted, or changed while restored — a restored instance counts by what it was built from, even after its saved document changes outside the API, until a restart or reinitialization rebuilds it, and with the document's current version a replace applies to it while this PyRIT can still use the document and a delete by its name (or by its document name when this PyRIT cannot use the document) removes it (an instance is taken for an initializer's only when it was not built from a saved recipe) — and while a saved document cannot be read from storage such a delete or change is refused (503), because it may reference the instance; a saved instance whose type is no longer registered counts as referencing every name its parameter values hold, and a saved document this PyRIT cannot use (for example one a newer PyRIT wrote) counts as referencing every name in its fields other thanschema_version,kind,name, andtypewhen a saved instance it names is deleted; an administrator can delete such a document by its instance name or its document name, even while saved instances reference it, and create the instance again — unless an instance built from it before it changed is still live and others hold it, which is refused like any live instance until they change or a restart rebuilds them. A saved instance that cannot be rebuilt (unset variable, unknown type, name taken by an initializer, failed dependency, unreadable or newer-format document) is listed with its reason inunrestorable, andGETreturns that reason when no live instance holds the name, instead of vanishing; instances that reference it are not rebuilt against another instance that now holds its name, and saving such a reference is refused (409) until it is deleted — as is any reference while the last restore failed as a whole (restore_error), and any create or replace while the store also still cannot be listed (503) — so what the API accepts is what the next restart rebuilds. A document is listed under its document name, whichDELETEaccepts (a name of that shape always means the document itself), when it does not name a valid, unreserved instance that maps back to it, and files whose names do not have the shape of a document name are ignored. A create from an older client that sends no name is saved under its generatedcompat_…name and can be deleted like any other.Commits (each passes its own tests):
MAINTExtractFileDocumentStoragefromCustomInitializerStorage(verbatim from FEAT: Add ScenarioPreset #2931 at8959c7b2d, conditional writes and operating-system locks included)FEATConditional deletes forFileDocumentStorage, on FEAT: Add ScenarioPreset #2931's conditional-write primitivesFEATRecipe models and storage, a largerget_sensitive_parameter_names,CredentialNames,UrlCredentialsFEATRestore-order plannerFIXMask credentials inHTTPTargetidentifier endpoints[BREAKING] FEATWrite-through persistence service, routes (PUT/DELETE), lifecycle hooksFEATGUI: target name, environment-variable fields, delete, unrestorable lists; e2e backend store isolated underdbdata/DOCConfiguration, GUI, backend README, registry docsWhy breaking. A create request that sends a credential parameter as a value (for example
params.api_key), or a value that carries a credential, now gets 400 (with identity authentication anapi_keyvalue is still dropped, as before); deleting a saved instance requires?version=; naming an environment variable requires administrator access, so only an administrator can create anHTTPTarget, whose requiredhttp_requestmust be a reference; a target's identifier no longer includes credentials from anHTTPTargetrequest URL; a scorer can no longer be namedtypes, whichGET /api/scorers/typeswould shadow. Storing raw keys at rest is what the story rules out, so these requests cannot keep working unchanged.Coordination.
8959c7b2d, whose conditional writes and per-document locks this PR uses instead of its own, so whichever lands first, the other drops its copy; commit 2 only adds a conditional delete on those primitives. That head replaced the age-based stale-lock recovery Rich flagged with operating-system locks, which the system releases when a writer exits, so no live writer can lose its lock. A local read of a saved document takes that document's lock (Windows cannot replace an open file), while a read of a name with nothing saved takes none, since FEAT: Add ScenarioPreset #2931's lock files stay in place and would otherwise accumulate; a listing reads without locks, as FEAT: Add ScenarioPreset #2931's does, since every replacement is atomic.Parameter.sensitive,multiline, andidentity_conflictingflags andget_auth_mode_parameters.sensitivehere also means "send a reference, never the value", soComponentIdentifier.get_sensitive_parameter_names()gains the parameters that can carry a credential (azure_speech_key,cookie,headers,http_request; its pinned-set test is updated). The server enforces the flags from the type's catalog parameters, as the GUI does, so a constructor parameter that a subclass identifier marks sensitive or identity-conflicting is enforced too (a registry reference, which names another instance, is never a credential). The target dialog renders every sensitive parameter, the multilinehttp_requestincluded, as an environment-variable input that sendscredentials(asCreateConverterDialogdoes, viafrontend/src/utils/credentialReference.ts), and identity authentication still clears the identity-conflicting ones.TargetService.build_asyncappliesget_auth_mode_parametersat every build, so a restored identity-modeAzureBlobStorageTargetkeeps itsauth_mode. Two behaviors change: with identity authentication, a value or reference for any parameter the catalog marksidentity_conflictingis refused (400) where FEAT: Add custom params to new targets in CoPYRIT #2846 dropped values, andheadersis now marked too, since a map can carry anAuthorizationheader, so the dialog clears it under identity as it doessas_token.Open questions for maintainers (defaults chosen): admin for credential references only (vs. every mutation);
headers/http_requestreference-only, so even a credential-free header map comes from a variable (vs. accepting values and rejecting only credential-looking entries, as free-form settings such ashttpx_client_kwargs.default_headersalready are); update of a referenced live instance refused (vs. cascade); initializer wins a name collision; environment-variable references only (no direct Key Vault kind); whether replacing or deleting a target should run its own cleanup (for example closing a realtime target's connections), which nothing in PyRIT calls today; restoring saved instances in the backend only (the storage and restore planner live inpyrit/registry, so notebooks andpyrit_scancould restore them in a follow-up).Tests and Documentation
New tests:
tests/unit/registry/test_{file_document_storage,instance_recipe_storage,instance_restore,sensitive_parameter_names}.py,tests/unit/backend/test_{instance_persistence_service,instance_credentials}.py,tests/unit/common/test_{credential_names,url_credentials}.py,tests/unit/prompt_target/target/test_http_target.py(identifier masking),tests/integration/registry/test_instance_recipe_storage_integration.py(real Blob, gated), Jest for the target and converter dialogs, targets page, converter registry,UnrestorableInstances,credentialReference, and the API client; existing backend, registry, and Playwright suites updated.Docs:
doc/getting_started/pyrit_conf.md,.pyrit_conf_example,doc/gui/0_gui.md(creating and saved targets, registry API migration notes),pyrit/backend/README.md,doc/code/registry/0_registry.md,frontend/README.md(local end-to-end runs save what they create). No notebooks changed, so JupyText was not run.