Repository navigation
[BREAKING] FIX: Keep uploads from API-created HTTP targets in a server-chosen folder - #3172
Open
varunj-msft wants to merge 1 commit into
Open
varunj-msft wants to merge 1 commit into
varunj-msft wants to merge 1 commit into
Conversation
…folder An HTTPXAPITarget created through the API could upload any file the server can read: the caller chose file_path and allowed_upload_directory, and a text message naming a path was uploaded too. The server now sets allowed_upload_directory from the new target_upload_directory setting, callers cannot supply it, and the type is left out of the target catalog while the setting is unset.
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
HTTPXAPITargetcan upload a local file when it sends: itsfile_path, a path-typed message piece, or a text prompt that names an existing file (deprecated). When it was created throughPOST /api/targets, the caller chose both the file andallowed_upload_directory, or left the folder unset, which allows any file. So an API caller could make the server upload any file it can read, such as.env, to a URL of their choice.Expected behavior:
target_upload_directorysetting in.pyrit_conf. The server passes it to the target asallowed_upload_directory, and callers can't set it (400).HTTPXAPITargetis not listed inGET /api/targets/types, and creating it through the API returns 400._validate_upload_path, so absolute paths,.., and symlinks that end up outside the folder are refused before anything is sent. A send like that records the target's error in the conversation, like other target errors; nothing is uploaded.PromptTarget.upload_directory_parameter(defaultNone) names the constructor parameter the server sets;HTTPXAPITargetsets it toallowed_upload_directory. The loader only checks that the setting is a non-empty string. The target checks that the folder exists when it is created, so a folder mounted after startup works, and a missing one fails that request instead of backend startup.BREAKING: API callers who create
HTTPXAPITargetnow need the operator to settarget_upload_directory, and can no longer passallowed_upload_directory.Not included: network destinations. Callers still choose
http_url, like any other target endpoint.Note for #3030 and #3058: both add other ways to build targets from API input (
build_from_source_async, and rebuilding saved targets at startup). They'll need the same folder injection. For #3058 the folder should be applied when the target is built, like the auth-mode parameters, rather than saved with the target.Tests and Documentation
test_target_service.py: the type is hidden without the setting and listed withoutallowed_upload_directorywhen it is set; a caller-supplied folder is rejected before construction; the server folder is injected.test_api_routes.py: the routes pass the configured folder to the service.test_runtime_lifecycle.py,test_configuration_loader.py: the setting is loaded and validated, published by the runtime, and left unset when startup fails..pyrit_conf_exampledocumentstarget_upload_directory.No JupyText changes.