Repository navigation
FEAT: Save targets created through the API (part 8) - #4026
Open
varunj-msft wants to merge 236 commits into
Open
varunj-msft wants to merge 236 commits into
varunj-msft wants to merge 236 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
The runtime status keeps its short summary. The restore log now also lists each field's full location and message, escaped, without the stored values, and with URL credentials removed. Credentials are removed before a location is shortened, so the summary can't keep part of one either.
# Conflicts: # pyrit/registry/registry_recipe_storage.py
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.
A suite launch runs its launch scripts during preflight and sends its scans without them, so the clash is raised there. Map it to 409 with its message, as a single launch does, and drop the per-entry message that a clash can no longer reach.
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 #4025, which refuses saved targets that would authenticate with a key. Until the earlier PRs merge, the diff here also shows their changes.
Problem. A target created through
POST /api/targetsis gone when the backend restarts. Restore (#3909) brings saved entries back at every start, but nothing saves one yet, and the target list doesn't say which targets will come back.Change.
POST /api/targetstakes"persist": true, which needsregisterleft on and a name that starts with a lowercase letter and has at most 64 lowercase letters, digits and underscores (otherwise 422). The target is built as before, saved as a recipe underregistry_recipes_source(by default~/.pyrit/registry_recipes, so no Azure is needed) with the signed-in user recorded as the one who saved it, and then registered. If registering fails, the file is deleted again; if that delete fails too, the request gets 503 and the server log names the target and its folder, so the file can be removed.headersis set to"{}", so a saved one doesn't send this server'sOPENAI_ADDITIONAL_REQUEST_HEADERS. Then the request goes through the same check restore uses, so a target that would hold or use a key, or leave an address to this server's settings, gets 400, and the message says it can still be created without saving it. A saved target can use saved targets and targets from configuration, which keep their own sign-in, but not a Temporary target, such as a round-robin member that won't come back after a restart.origin:saved,configuration(registered by the server's startup scripts), ortemporary(gone after a restart or reinitialization, such as a target created withoutpersistor registered by a scan's launch scripts).registry_recipes_sourceat a new temp folder, so they don't read or write the targets saved on the machine running them.What stays the same. Creating a target without
persistworks as before, and permission checks are unchanged and come first, so a non-admin whose target would use a server credential still gets 403. A save runs inside the create request, and apply already reports busy while a request is in flight, so no new lock is needed. Restore is unchanged, except that it first marks the targets the startup scripts registered asconfiguration.Not covered.
GET /api/config/runtime. That can still happen after an upgrade or a change to the environment.Tests and Documentation
tests/unit/backend/test_target_service.py: an Entra ID target is saved and restore rebuilds it; a no-key target is saved without an owner when sign-in is off; a target that would hold or use a key isn't saved or registered; a taken name is a conflict only when saving; a saved file with the name is a conflict and stays as it was; the file is deleted when the target can't be registered; storage failures are reported as unavailable, and the log names the target, its folder and the error type without the error's text; two saves of one name save one target; a round-robin is saved only over targets that come back after a restart; a class registered under another name is saved so restore finds it; a failed save that can't be undone is reported as storage trouble; a reserved name is refused as invalid, not as taken; the list shows where each target comes from.tests/unit/backend/test_api_routes.py: a save conflict maps to 409 and a storage failure to 503; the saved-entries source and the signed-in owner (or none when sign-in is off) reach the service;persistneedsregisterand a valid name.tests/unit/backend/test_mappers.py: the origin of each registered target, and on the instance.tests/unit/backend/test_runtime_lifecycle.py: startup marks the targets its scripts register but not saved ones; a save in flight makes apply report busy.tests/unit/registry/test_target_registry.py: a class a launch script adds, or puts in place of another under an existing name, is temporary.tests/unit/registry/test_instance_registry.pyandtest_registry_recipe_storage.py: startup entries are marked and keep the mark when replaced; a recipe is created once, lists back with its owner and version, and is deleted once.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,488 passed, 9 skipped, 4 xfailednpm run lintandnpm run type-check(forplaywright.config.ts): passed.E2E_SEEDED_MODE=true npx playwright test --project=seeded --workers=1: 108 passed, with the backend started from the generated temp config.saved); the same name gets 409; a typed key, an empty key and an endpoint with?sig=get 400 without echoing the key; exactly one recipe file is written, with no key. After a restart the saved target is back and answers a chat message, and the Temporary one is gone.pyrit/backend/README.mdanddoc/getting_started/pyrit_conf.md: how to save a target, the name rule, what it can hold, theheadersdefault, the OpenAI SDK settings that still apply, and the status codes.No JupyText changes.