diff --git a/api/agent-notes/controllers/files/__init__.py.md b/api/agent-notes/controllers/files/__init__.py.md index 5e4fe3a108..bbe9598183 100644 --- a/api/agent-notes/controllers/files/__init__.py.md +++ b/api/agent-notes/controllers/files/__init__.py.md @@ -1,6 +1,6 @@ Summary: - Registers file-related API namespaces and routes for files service. -- Includes app-assets download proxy controller. +- Includes app-assets download/upload proxy controllers. Invariants: - files_ns must include all file controller modules to register routes. diff --git a/api/agent-notes/controllers/files/app_assets_upload.py.md b/api/agent-notes/controllers/files/app_assets_upload.py.md new file mode 100644 index 0000000000..e58fc0f68d --- /dev/null +++ b/api/agent-notes/controllers/files/app_assets_upload.py.md @@ -0,0 +1,13 @@ +Summary: +- App assets upload proxy endpoint (signed URL verification, upload to storage). + +Invariants: +- Validates AssetPath fields (UUIDs, asset_type allowlist). +- Verifies tenant-scoped signature and expiration before writing storage. +- URL uses expires_at/nonce/sign query params. + +Edge Cases: +- Invalid signature or expired link returns Forbidden. + +Tests: +- Verify signature validation and invalid/expired cases. diff --git a/api/agent-notes/core/app_assets/storage.py.md b/api/agent-notes/core/app_assets/storage.py.md index 0c073becd1..ee488e3c62 100644 --- a/api/agent-notes/core/app_assets/storage.py.md +++ b/api/agent-notes/core/app_assets/storage.py.md @@ -13,6 +13,7 @@ Invariants: Edge Cases: - Storage backends without presign support must fall back to signed proxy URLs. - Signed proxy verification enforces expiration and tenant-scoped signing keys. +- Upload URLs also fall back to signed proxy endpoints when presign is unsupported. - load_or_none treats SilentStorage "File Not Found" bytes as missing. Tests: diff --git a/api/controllers/files/__init__.py b/api/controllers/files/__init__.py index 854cf3cbf8..c2e5e52bec 100644 --- a/api/controllers/files/__init__.py +++ b/api/controllers/files/__init__.py @@ -14,13 +14,14 @@ api = ExternalApi( files_ns = Namespace("files", description="File operations", path="/") -from . import app_assets_download, image_preview, storage_download, tool_files, upload +from . import app_assets_download, app_assets_upload, image_preview, storage_download, tool_files, upload api.add_namespace(files_ns) __all__ = [ "api", "app_assets_download", + "app_assets_upload", "bp", "files_ns", "image_preview", diff --git a/api/controllers/files/app_assets_upload.py b/api/controllers/files/app_assets_upload.py new file mode 100644 index 0000000000..92657d815a --- /dev/null +++ b/api/controllers/files/app_assets_upload.py @@ -0,0 +1,60 @@ +from flask import Response, request +from flask_restx import Resource +from pydantic import BaseModel, Field +from werkzeug.exceptions import Forbidden + +from controllers.files import files_ns +from core.app_assets.storage import AppAssetSigner, AssetPath, app_asset_storage + +DEFAULT_REF_TEMPLATE_SWAGGER_2_0 = "#/definitions/{model}" + + +class AppAssetUploadQuery(BaseModel): + expires_at: int = Field(..., description="Unix timestamp when the link expires") + nonce: str = Field(..., description="Random string for signature") + sign: str = Field(..., description="HMAC signature") + + +files_ns.schema_model( + AppAssetUploadQuery.__name__, + AppAssetUploadQuery.model_json_schema(ref_template=DEFAULT_REF_TEMPLATE_SWAGGER_2_0), +) + + +@files_ns.route("/app-assets/////upload") +@files_ns.route( + "/app-assets//////upload" +) +class AppAssetUploadApi(Resource): + def put( + self, + asset_type: str, + tenant_id: str, + app_id: str, + resource_id: str, + sub_resource_id: str | None = None, + ): + args = AppAssetUploadQuery.model_validate(request.args.to_dict(flat=True)) + + try: + asset_path = AssetPath.from_components( + asset_type=asset_type, + tenant_id=tenant_id, + app_id=app_id, + resource_id=resource_id, + sub_resource_id=sub_resource_id, + ) + except ValueError as exc: + raise Forbidden(str(exc)) from exc + + if not AppAssetSigner.verify_upload_signature( + asset_path=asset_path, + expires_at=args.expires_at, + nonce=args.nonce, + sign=args.sign, + ): + raise Forbidden("Invalid or expired upload link") + + content = request.get_data() + app_asset_storage.save(asset_path, content) + return Response(status=204) diff --git a/api/core/app_assets/storage.py b/api/core/app_assets/storage.py index 9a1f5fbaf2..d37d4cf1b8 100644 --- a/api/core/app_assets/storage.py +++ b/api/core/app_assets/storage.py @@ -162,6 +162,7 @@ class AppAssetSigner: SIGNATURE_PREFIX = "app-asset" SIGNATURE_VERSION = "v1" OPERATION_DOWNLOAD = "download" + OPERATION_UPLOAD = "upload" @classmethod def create_download_signature(cls, asset_path: AssetPathBase, expires_at: int, nonce: str) -> str: @@ -172,13 +173,51 @@ class AppAssetSigner: nonce=nonce, ) + @classmethod + def create_upload_signature(cls, asset_path: AssetPathBase, expires_at: int, nonce: str) -> str: + return cls._create_signature( + asset_path=asset_path, + operation=cls.OPERATION_UPLOAD, + expires_at=expires_at, + nonce=nonce, + ) + @classmethod def verify_download_signature(cls, asset_path: AssetPathBase, expires_at: int, nonce: str, sign: str) -> bool: + return cls._verify_signature( + asset_path=asset_path, + operation=cls.OPERATION_DOWNLOAD, + expires_at=expires_at, + nonce=nonce, + sign=sign, + ) + + @classmethod + def verify_upload_signature(cls, asset_path: AssetPathBase, expires_at: int, nonce: str, sign: str) -> bool: + return cls._verify_signature( + asset_path=asset_path, + operation=cls.OPERATION_UPLOAD, + expires_at=expires_at, + nonce=nonce, + sign=sign, + ) + + @classmethod + def _verify_signature( + cls, + *, + asset_path: AssetPathBase, + operation: str, + expires_at: int, + nonce: str, + sign: str, + ) -> bool: if expires_at <= 0: return False - expected_sign = cls.create_download_signature( + expected_sign = cls._create_signature( asset_path=asset_path, + operation=operation, expires_at=expires_at, nonce=nonce, ) @@ -269,7 +308,7 @@ class AppAssetStorage: except NotImplementedError: pass - return self._generate_signed_proxy_url(asset_path, expires_in, for_external=for_external) + return self._generate_signed_proxy_download_url(asset_path, expires_in, for_external=for_external) def get_download_urls( self, @@ -287,30 +326,56 @@ class AppAssetStorage: pass return [ - self._generate_signed_proxy_url(asset_path, expires_in, for_external=for_external) + self._generate_signed_proxy_download_url(asset_path, expires_in, for_external=for_external) for asset_path in asset_paths_list ] - # FIXME(Mairuis): support fallback to signed proxy url - def get_upload_url(self, asset_path: AssetPathBase, expires_in: int = 3600) -> str: - return self._storage.get_upload_url(self.get_storage_key(asset_path), expires_in) + def get_upload_url( + self, + asset_path: AssetPathBase, + expires_in: int = 3600, + *, + for_external: bool = True, + ) -> str: + storage_key = self.get_storage_key(asset_path) + try: + return self._storage.get_upload_url(storage_key, expires_in) + except NotImplementedError: + pass - def _generate_signed_proxy_url(self, asset_path: AssetPathBase, expires_in: int, *, for_external: bool) -> str: + return self._generate_signed_proxy_upload_url(asset_path, expires_in, for_external=for_external) + + def _generate_signed_proxy_download_url( + self, asset_path: AssetPathBase, expires_in: int, *, for_external: bool + ) -> str: expires_in = min(expires_in, dify_config.FILES_ACCESS_TIMEOUT) expires_at = int(time.time()) + max(expires_in, 1) nonce = os.urandom(16).hex() sign = AppAssetSigner.create_download_signature(asset_path=asset_path, expires_at=expires_at, nonce=nonce) base_url = dify_config.FILES_URL if for_external else (dify_config.INTERNAL_FILES_URL or dify_config.FILES_URL) - url = self._build_proxy_url(base_url=base_url, asset_path=asset_path) + url = self._build_proxy_url(base_url=base_url, asset_path=asset_path, action="download") + query = urllib.parse.urlencode({"expires_at": expires_at, "nonce": nonce, "sign": sign}) + return f"{url}?{query}" + + def _generate_signed_proxy_upload_url( + self, asset_path: AssetPathBase, expires_in: int, *, for_external: bool + ) -> str: + expires_in = min(expires_in, dify_config.FILES_ACCESS_TIMEOUT) + expires_at = int(time.time()) + max(expires_in, 1) + nonce = os.urandom(16).hex() + sign = AppAssetSigner.create_upload_signature(asset_path=asset_path, expires_at=expires_at, nonce=nonce) + + base_url = dify_config.FILES_URL if for_external else (dify_config.INTERNAL_FILES_URL or dify_config.FILES_URL) + url = self._build_proxy_url(base_url=base_url, asset_path=asset_path, action="upload") query = urllib.parse.urlencode({"expires_at": expires_at, "nonce": nonce, "sign": sign}) return f"{url}?{query}" @staticmethod - def _build_proxy_url(*, base_url: str, asset_path: AssetPathBase) -> str: + def _build_proxy_url(*, base_url: str, asset_path: AssetPathBase, action: str) -> str: encoded_parts = [urllib.parse.quote(part, safe="") for part in asset_path.proxy_path_parts()] path = "/".join(encoded_parts) - return f"{base_url}/files/app-assets/{path}/download" + return f"{base_url}/files/app-assets/{path}/{action}" class _LazyAppAssetStorage: diff --git a/api/tests/unit_tests/extensions/otel/decorators/test_base.py b/api/tests/unit_tests/extensions/otel/decorators/test_base.py index a42f861bb7..d28f351b1b 100644 --- a/api/tests/unit_tests/extensions/otel/decorators/test_base.py +++ b/api/tests/unit_tests/extensions/otel/decorators/test_base.py @@ -24,7 +24,7 @@ class TestTraceSpanDecorator: """Test that decorated function executes and returns correct value.""" @trace_span() - def test_func(x, y): + def test_func(x, y): # noqa: FURB118 return x + y result = test_func(2, 3)