Repository navigation
FEAT: Show who saved each target and list only your own (part 10) - #4028
Open
varunj-msft wants to merge 175 commits into
Open
varunj-msft wants to merge 175 commits into
varunj-msft wants to merge 175 commits into
Conversation
…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
…benchmark's recipes folder
…d to OpenAIChatTarget
varunj-msft
changed the title
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.
Listing catches only storage errors, so a bug is raised instead of being reported as unreadable storage, and a bug while planning is a build error rather than an invalid recipe. Each failure is logged once, with the error behind it and URL credentials removed. The report shows the error's text with each URL's user name, query and fragment removed, any SAS signature hidden, and the length capped. A class that moved within PyRIT is restored; one now registered from outside PyRIT still fails. A technique whose name or tag clashes is a name clash, and anything else registration refuses is an invalid recipe. A recipe that sets a server file or folder is refused before it is built. Class metadata comes from the registry's cache. The class docstring lists the steps the API's create runs that restore skips.
The runtime becomes ready even when some saved entries fail; the failures stay in the runtime status. Each failed entry keeps its name in its registry: looking it up, or taking the name, raises SavedEntryUnavailableError, which the backend answers with 409, so nothing else stands in for it. Administrators see the full message and everyone else a fixed explanation of the reason. A folder that can't be listed holds every unregistered name of its kind, and each entry it left unbuilt is reported. While a saved technique is held, a launch or estimate that picks techniques by all, default or a tag is refused with 409 for scenarios whose techniques come from the registry, since the broken technique's tags may be unknown. Naming techniques still works; naming the held one fails with its reason. Route catch-alls let the error through instead of turning it into a 500.
A launch script that registers a name held for a saved entry that couldn't be restored is refused, and the refusal is recorded even when the script catches it, so the launch stops instead of running with the script's entry in the saved one's place. A suite entry that fails on a held entry, or on a broad technique selection, shows the reason without the error's details.
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.
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
This is one of the changes for saving Registry entries across restarts. It builds on #4027, which saves targets from the Registry. Until the earlier PRs merge, the diff here also shows their changes.
Problem. With sign-in on, everyone's saved targets land in one Targets list, and nothing says who saved which, so users can't find their own. Frederic raised this on the design gist and suggested Mine, Official and All views.
Change.
saved_by, the name) and whether that's the signed-in user asking (saved_by_me). The server compares the object IDs, so no ID leaves it.What stays the same. Anyone can still save and use every target, and nothing here decides who can change or delete one. The saved file format doesn't change. Targets from configuration and Temporary targets have no owner. The round robin member list and the other target pickers are unchanged.
Not covered.
Tests and Documentation
tests/unit/registry/test_registry_recipe_restore.py: restore keeps who saved each target, converter and scorer; an entry saved without sign-in has nobody.tests/unit/backend/test_target_service.py: a saved target says who saved it and whether it was the caller, for the saver, another user and nobody signed in, the same before and after a restart; a target saved with sign-in off and a Temporary one name nobody, including the create response.tests/unit/backend/test_mappers.py: the name and the flag for each owner and caller, a blank name, no object ID in the response, and inner targets of a saved round robin naming nobody.tests/unit/backend/test_api_routes.pyandtest_auth_middleware.py: list and get pass the caller's object ID, and none with sign-in off.TargetTable.test.tsx: the captions; the checkbox narrowing the list, together with the column filters and with hidden targets; no checkbox when nothing is yours; turning off when your last saved target is gone.frontend/e2e/config.spec.ts(mock): the captions, the checkbox, and a fresh load listing everyone again.frontend/e2e/registry.spec.ts(seeded, sign-in off): nobody is named and there's no checkbox.uv run --frozen --extra all python -m pytest -n 8 --dist=loadfile tests/unit/registry tests/unit/backend tests/unit/prompt_target tests/unit/models tests/unit/setup tests/unit/converter tests/unit/score: 19,612 passed, 9 skipped, 4 xfailedcd frontend && npm run test:coverage: 184 suites, 4,563 tests passedcd frontend && npx playwright test --project=mock e2e/config.spec.ts e2e/registry.spec.ts: 29 passedcd frontend && E2E_SEEDED_MODE=true npx playwright test --project=seeded: 109 passednpx tsc --noEmit,npx eslinton the changed files, and pre-commit (ruff, ty, docs checks): passeddoc/gui/0_gui.md(Registry): the "Saved by" caption and the checkbox.pyrit/backend/README.md: the two new fields.No JupyText changes.