diff --git a/api/controllers/inner_api/__init__.py b/api/controllers/inner_api/__init__.py index f47861cf274..277e0f5ec0e 100644 --- a/api/controllers/inner_api/__init__.py +++ b/api/controllers/inner_api/__init__.py @@ -17,6 +17,7 @@ inner_api_ns = Namespace("inner_api", description="Internal API operations", pat from . import mail as _mail from . import runtime_credentials as _runtime_credentials +from .agent import files as _agent_files from .agent import tools as _agent_tools from .app import dsl as _app_dsl from .knowledge import retrieval as _knowledge_retrieval @@ -30,6 +31,7 @@ api.add_namespace(inner_api_ns) __all__ = [ "_agent_config", "_agent_drive", + "_agent_files", "_agent_tools", "_app_dsl", "_knowledge_retrieval", diff --git a/api/controllers/inner_api/agent/files.py b/api/controllers/inner_api/agent/files.py new file mode 100644 index 00000000000..7a5cad76440 --- /dev/null +++ b/api/controllers/inner_api/agent/files.py @@ -0,0 +1,198 @@ +"""Agent-owned inner endpoints for CLI file URL allocation.""" + +from __future__ import annotations + +from typing import Literal + +from flask_restx import Resource +from pydantic import BaseModel, ConfigDict, ValidationError +from sqlalchemy.orm import Session + +from configs import dify_config +from controllers.common.schema import register_response_schema_models, register_schema_models +from controllers.common.session import with_session +from controllers.console.wraps import setup_required +from controllers.inner_api import inner_api_ns +from controllers.inner_api.plugin.wraps import get_user +from controllers.inner_api.wraps import plugin_inner_api_only +from core.plugin.entities.request import RequestDownloadFileMapping, RequestRequestUploadFile +from core.tools.signature import bind_file_uri, get_signed_file_uri_for_plugin +from fields.base import ResponseModel +from libs.exception import BaseHTTPException +from services.account_service import TenantService +from services.file_request_service import FileRequestService + + +class AgentFileRequestHttpError(BaseHTTPException): + error_code = "agent_file_request_failed" + description = "Agent file request failed." + code = 500 + + def __init__(self, *, error_code: str, description: str, status_code: int) -> None: + self.error_code = error_code + self.description = description + self.code = status_code + super().__init__(description) + + +class AgentFileUploadRequestPayload(RequestRequestUploadFile): + tenant_id: str + user_id: str + + model_config = ConfigDict(extra="forbid") + + +class AgentFileDownloadRequestPayload(BaseModel): + tenant_id: str + user_id: str + user_from: Literal["account", "end-user"] + invoke_from: Literal[ + "service-api", + "openapi", + "web-app", + "trigger", + "explore", + "debugger", + "published", + "validation", + ] + file: RequestDownloadFileMapping + for_frontend: bool = True + + model_config = ConfigDict(extra="forbid") + + +class AgentFileUploadRequestResponse(ResponseModel): + upload_uri: str + + +class AgentFileDownloadRequestResponse(ResponseModel): + filename: str + mime_type: str | None = None + size: int + download_uri: str + + +register_schema_models(inner_api_ns, AgentFileUploadRequestPayload, AgentFileDownloadRequestPayload) +register_response_schema_models( + inner_api_ns, + AgentFileUploadRequestResponse, + AgentFileDownloadRequestResponse, +) + + +@inner_api_ns.route("/agent/files/upload-request") +class AgentFileUploadRequestApi(Resource): + """Allocate an origin-free signed upload URI for the Agent CLI.""" + + @setup_required + @plugin_inner_api_only + @inner_api_ns.doc("inner_agent_file_upload_request") + @inner_api_ns.expect(inner_api_ns.models[AgentFileUploadRequestPayload.__name__]) + @inner_api_ns.response( + 200, + "Upload URI allocated", + inner_api_ns.models[AgentFileUploadRequestResponse.__name__], + ) + @with_session(write=False) + def post(self, session: Session) -> dict[str, object]: + try: + payload = AgentFileUploadRequestPayload.model_validate(inner_api_ns.payload or {}) + except ValidationError as exc: + raise AgentFileRequestHttpError( + error_code="invalid_request", + description=str(exc), + status_code=400, + ) from exc + + tenant = TenantService.get_tenant_by_id(payload.tenant_id, session=session) + if tenant is None: + raise AgentFileRequestHttpError( + error_code="tenant_not_found", + description="tenant not found", + status_code=404, + ) + try: + user = get_user(tenant.id, payload.user_id) + upload_uri = get_signed_file_uri_for_plugin( + filename=payload.filename, + mimetype=payload.mimetype, + tenant_id=tenant.id, + user_id=user.id, + conversation_id=payload.conversation_id, + ) + except ValueError as exc: + raise AgentFileRequestHttpError( + error_code="user_not_found", + description=str(exc), + status_code=404, + ) from exc + + return AgentFileUploadRequestResponse(upload_uri=upload_uri).model_dump(mode="json") + + +@inner_api_ns.route("/agent/files/download-request") +class AgentFileDownloadRequestApi(Resource): + """Allocate a transfer URI or frontend URL for one Agent CLI file.""" + + @setup_required + @plugin_inner_api_only + @inner_api_ns.doc("inner_agent_file_download_request") + @inner_api_ns.expect(inner_api_ns.models[AgentFileDownloadRequestPayload.__name__]) + @inner_api_ns.response( + 200, + "Download URI allocated", + inner_api_ns.models[AgentFileDownloadRequestResponse.__name__], + ) + @with_session(write=False) + def post(self, session: Session) -> dict[str, object]: + try: + payload = AgentFileDownloadRequestPayload.model_validate(inner_api_ns.payload or {}) + except ValidationError as exc: + raise AgentFileRequestHttpError( + error_code="invalid_request", + description=str(exc), + status_code=400, + ) from exc + + if TenantService.get_tenant_by_id(payload.tenant_id, session=session) is None: + raise AgentFileRequestHttpError( + error_code="tenant_not_found", + description="tenant not found", + status_code=404, + ) + try: + result = FileRequestService().request_download( + tenant_id=payload.tenant_id, + user_id=payload.user_id, + user_from=payload.user_from, + invoke_from=payload.invoke_from, + file_mapping=payload.file.model_dump(mode="python", exclude_none=True), + ) + except ValueError as exc: + raise AgentFileRequestHttpError( + error_code="file_not_accessible", + description=str(exc), + status_code=404, + ) from exc + + download_uri = result.download_uri + if payload.for_frontend: + download_uri = bind_file_uri(download_uri, dify_config.FILES_URL) + + return AgentFileDownloadRequestResponse( + filename=result.filename, + mime_type=result.mime_type, + size=result.size, + download_uri=download_uri, + ).model_dump(mode="json") + + +__all__ = [ + "AgentFileDownloadRequestApi", + "AgentFileDownloadRequestPayload", + "AgentFileDownloadRequestResponse", + "AgentFileUploadRequestApi", + "AgentFileUploadRequestPayload", + "AgentFileUploadRequestResponse", +] diff --git a/api/controllers/inner_api/plugin/plugin.py b/api/controllers/inner_api/plugin/plugin.py index 1171761fc53..221887f73c7 100644 --- a/api/controllers/inner_api/plugin/plugin.py +++ b/api/controllers/inner_api/plugin/plugin.py @@ -1,6 +1,7 @@ from flask_restx import Resource from sqlalchemy.orm import Session +from configs import dify_config from controllers.console.app.wraps import with_session from controllers.console.wraps import setup_required from controllers.inner_api import inner_api_ns @@ -31,7 +32,7 @@ from core.plugin.entities.request import ( RequestRequestUploadFile, ) from core.tools.entities.tool_entities import ToolProviderType -from core.tools.signature import get_signed_file_url_for_plugin +from core.tools.signature import bind_file_uri, get_signed_file_uri_for_plugin from extensions.ext_database import db from graphon.model_runtime.utils.encoders import jsonable_encoder from libs.helper import length_prefixed_response @@ -429,13 +430,14 @@ class PluginUploadFileRequestApi(Resource): ) def post(self, user_model: Account | EndUser, tenant_model: Tenant, payload: RequestRequestUploadFile): # generate signed url - url = get_signed_file_url_for_plugin( + uri = get_signed_file_uri_for_plugin( filename=payload.filename, mimetype=payload.mimetype, tenant_id=tenant_model.id, user_id=user_model.id, conversation_id=payload.conversation_id, ) + url = bind_file_uri(uri, dify_config.INTERNAL_FILES_URL or dify_config.FILES_URL) return BaseBackwardsInvocationResponse(data={"url": url}).model_dump() @@ -454,36 +456,33 @@ class PluginDownloadFileRequestApi(Resource): } ) def post(self, payload: RequestRequestDownloadFile): - """Resolve signed download metadata for trusted external runtimes. + """Adapt a shared file request result to the Plugin backward contract. - Unlike end-user-facing upload/download APIs, this inner endpoint serves - trusted callers such as the ``dify-agent`` back proxy. The caller sends - flattened ``tenant_id`` / ``user_id`` / ``user_from`` / ``invoke_from`` - context explicitly in the body, and ``FileRequestService`` rebuilds the - corresponding ``FileAccessScope`` before resolving the signed URL. - - The response is control-plane metadata only: filename, mime type, size, - and the signed download URL. File bytes still flow through the existing - signed file endpoints rather than through this inner API. + ``FileRequestService`` rebuilds the caller's ``FileAccessScope`` and + resolves one origin-free signed URI. This controller binds that URI to + the Plugin-selected external or internal files base URL, then returns + the existing backward-invocation envelope. """ tenant_model = db.session.get(Tenant, payload.tenant_id) if tenant_model is None: raise ValueError("tenant not found") - result = FileRequestService().request_download_url( + result = FileRequestService().request_download( tenant_id=tenant_model.id, user_id=payload.user_id, user_from=payload.user_from, invoke_from=payload.invoke_from, file_mapping=payload.file.model_dump(mode="python", exclude_none=True), - for_external=payload.for_external, + ) + base_url = ( + dify_config.FILES_URL if payload.for_external else (dify_config.INTERNAL_FILES_URL or dify_config.FILES_URL) ) return BaseBackwardsInvocationResponse( data={ "filename": result.filename, "mime_type": result.mime_type, "size": result.size, - "download_url": result.download_url, + "download_url": bind_file_uri(result.download_uri, base_url), } ).model_dump() diff --git a/api/core/app/workflow/file_runtime.py b/api/core/app/workflow/file_runtime.py index dec92c03b61..7bdabd93717 100644 --- a/api/core/app/workflow/file_runtime.py +++ b/api/core/app/workflow/file_runtime.py @@ -13,7 +13,7 @@ from configs import dify_config from core.app.file_access import DatabaseFileAccessController, FileAccessControllerProtocol from core.db.session_factory import session_factory from core.file import remote_fetcher -from core.tools.signature import sign_tool_file +from core.tools.signature import bind_file_uri, sign_tool_file_uri from core.workflow.file_reference import parse_file_reference from extensions.ext_storage import storage from graphon.file import FileTransferMethod @@ -62,32 +62,42 @@ class DifyWorkflowFileRuntime(WorkflowFileRuntimeProtocol): @override def resolve_file_url(self, *, file: File, for_external: bool = True) -> str | None: + uri = self.resolve_file_uri(file=file) + if uri is None or file.transfer_method == FileTransferMethod.REMOTE_URL: + return uri + return bind_file_uri(uri, self._base_url(for_external=for_external)) + + def resolve_file_uri(self, *, file: File) -> str | None: + """Resolve a signed file URI without binding Dify-owned files to an origin. + + Remote URLs retain their external absolute URL. Dify-owned files return + a signed ``/files/...`` URI that callers can bind to their own network + audience without exposing ``FILES_URL`` or ``INTERNAL_FILES_URL``. + """ + if file.transfer_method == FileTransferMethod.REMOTE_URL: return file.remote_url parsed_reference = parse_file_reference(file.reference) if parsed_reference is None: raise ValueError("Missing file reference") if file.transfer_method == FileTransferMethod.LOCAL_FILE: - return self.resolve_upload_file_url( + return self.resolve_upload_file_uri( upload_file_id=parsed_reference.record_id, - for_external=for_external, ) if file.transfer_method == FileTransferMethod.DATASOURCE_FILE: if file.extension is None: raise ValueError("Missing file extension") self._assert_upload_file_access(upload_file_id=parsed_reference.record_id) - return sign_tool_file( + return sign_tool_file_uri( tool_file_id=parsed_reference.record_id, extension=file.extension, - for_external=for_external, ) if file.transfer_method == FileTransferMethod.TOOL_FILE: if file.extension is None: raise ValueError("Missing file extension") - return self.resolve_tool_file_url( + return self.resolve_tool_file_uri( tool_file_id=parsed_reference.record_id, extension=file.extension, - for_external=for_external, ) return None @@ -99,18 +109,34 @@ class DifyWorkflowFileRuntime(WorkflowFileRuntimeProtocol): as_attachment: bool = False, for_external: bool = True, ) -> str: + uri = self.resolve_upload_file_uri(upload_file_id=upload_file_id, as_attachment=as_attachment) + return bind_file_uri(uri, self._base_url(for_external=for_external)) + + def resolve_upload_file_uri( + self, + *, + upload_file_id: str, + as_attachment: bool = False, + ) -> str: + """Resolve a signed UploadFile URI without selecting an origin.""" + self._assert_upload_file_access(upload_file_id=upload_file_id) - base_url = self._base_url(for_external=for_external) - url = f"{base_url}/files/{upload_file_id}/file-preview" + uri = f"/files/{upload_file_id}/file-preview" query = self._sign_query(payload=f"file-preview|{upload_file_id}") if as_attachment: query["as_attachment"] = "true" - return f"{url}?{urllib.parse.urlencode(query)}" + return f"{uri}?{urllib.parse.urlencode(query)}" @override def resolve_tool_file_url(self, *, tool_file_id: str, extension: str, for_external: bool = True) -> str: + uri = self.resolve_tool_file_uri(tool_file_id=tool_file_id, extension=extension) + return bind_file_uri(uri, self._base_url(for_external=for_external)) + + def resolve_tool_file_uri(self, *, tool_file_id: str, extension: str) -> str: + """Resolve a signed ToolFile URI without selecting an origin.""" + self._assert_tool_file_access(tool_file_id=tool_file_id) - return sign_tool_file(tool_file_id=tool_file_id, extension=extension, for_external=for_external) + return sign_tool_file_uri(tool_file_id=tool_file_id, extension=extension) @override def verify_preview_signature( diff --git a/api/core/tools/signature.py b/api/core/tools/signature.py index 77d81bafbc1..02291bd2611 100644 --- a/api/core/tools/signature.py +++ b/api/core/tools/signature.py @@ -4,29 +4,52 @@ import hmac import os import time import urllib.parse +from urllib.parse import urlsplit from configs import dify_config +def bind_file_uri(uri: str, base_url: str) -> str: + """Bind a Dify-owned file URI to one caller-selected origin. + + Explicit remote HTTP(S) URLs are already complete and pass through. Other + values must be origin-free ``/files/...`` URIs. + """ + + parsed = urlsplit(uri) + if parsed.scheme in {"http", "https"} and parsed.netloc: + return uri + if ( + parsed.scheme + or parsed.netloc + or parsed.fragment + or uri.startswith("//") + or not parsed.path.startswith("/files/") + ): + raise ValueError("file URI must be an absolute HTTP(S) URL or a /files/ URI") + return f"{base_url}{uri}" + + def _secret_key() -> bytes: return dify_config.SECRET_KEY.encode() -def sign_tool_file(tool_file_id: str, extension: str, for_external: bool = True) -> str: - """ - sign file to get a temporary url for plugin access - """ - # Use internal URL for plugin/tool file access in Docker environments, unless for_external is True - base_url = dify_config.FILES_URL if for_external else (dify_config.INTERNAL_FILES_URL or dify_config.FILES_URL) - file_preview_url = f"{base_url}/files/tools/{tool_file_id}{extension}" - +def sign_tool_file_uri(tool_file_id: str, extension: str) -> str: + """Sign a ToolFile path without selecting a network origin.""" timestamp = str(int(time.time())) nonce = os.urandom(16).hex() data_to_sign = f"file-preview|{tool_file_id}|{timestamp}|{nonce}" sign = hmac.new(_secret_key(), data_to_sign.encode(), hashlib.sha256).digest() encoded_sign = base64.urlsafe_b64encode(sign).decode() - return f"{file_preview_url}?timestamp={timestamp}&nonce={nonce}&sign={encoded_sign}" + return f"/files/tools/{tool_file_id}{extension}?timestamp={timestamp}&nonce={nonce}&sign={encoded_sign}" + + +def sign_tool_file(tool_file_id: str, extension: str, for_external: bool = True) -> str: + """Sign a ToolFile URL for the browser or an internal Dify service.""" + + base_url = dify_config.FILES_URL if for_external else (dify_config.INTERNAL_FILES_URL or dify_config.FILES_URL) + return bind_file_uri(sign_tool_file_uri(tool_file_id, extension), base_url) def sign_upload_file_preview_url(upload_file_id: str, extension: str) -> str: @@ -64,13 +87,11 @@ def verify_tool_file_signature(file_id: str, timestamp: str, nonce: str, sign: s return current_time - int(timestamp) <= dify_config.FILES_ACCESS_TIMEOUT -def get_signed_file_url_for_plugin( +def get_signed_file_uri_for_plugin( filename: str, mimetype: str, tenant_id: str, user_id: str, conversation_id: str | None = None ) -> str: - """Build the signed upload URL used by the plugin-facing file upload endpoint.""" + """Build a signed plugin-upload URI without selecting a network origin.""" - base_url = dify_config.INTERNAL_FILES_URL or dify_config.FILES_URL - upload_url = f"{base_url}/files/upload/for-plugin" timestamp = str(int(time.time())) nonce = os.urandom(16).hex() data_to_sign = f"upload|{filename}|{mimetype}|{tenant_id}|{user_id}|{conversation_id or ''}|{timestamp}|{nonce}" @@ -86,7 +107,7 @@ def get_signed_file_url_for_plugin( if conversation_id: query_params["conversation_id"] = conversation_id query = urllib.parse.urlencode(query_params) - return f"{upload_url}?{query}" + return f"/files/upload/for-plugin?{query}" def verify_plugin_file_signature( diff --git a/api/core/workflow/nodes/agent_v2/runtime_request_builder.py b/api/core/workflow/nodes/agent_v2/runtime_request_builder.py index 09860998c89..126ce503f81 100644 --- a/api/core/workflow/nodes/agent_v2/runtime_request_builder.py +++ b/api/core/workflow/nodes/agent_v2/runtime_request_builder.py @@ -701,7 +701,7 @@ class WorkflowAgentRuntimeRequestBuilder: "only the accepted file-mapping shape and the returned `reference`; never invent the `reference` " "value.", "If you are replying to the user in natural language and want them to open or download the produced " - "file, include the returned `download_url` in that reply instead of copying it into structured " + "file, include the returned `public_download_url` in that reply instead of copying it into structured " "`final_output` unless the schema explicitly asks for it.", *file_output_lines, ] diff --git a/api/services/agent/prompt_mentions.py b/api/services/agent/prompt_mentions.py index 5f42bffe3ff..a3690f4093a 100644 --- a/api/services/agent/prompt_mentions.py +++ b/api/services/agent/prompt_mentions.py @@ -320,7 +320,7 @@ def _format_output_mention(output: DeclaredOutputConfig) -> str: f"{output.name} (file output; create the file locally, run " f"`dify-agent file upload `, then set final_output.{output.name} to a `tool_file` mapping " f"using the returned `reference`; if replying to the user in natural language, use the returned " - f"`download_url`; do not call final_output before upload succeeds, and do not use the local path, " + f"`public_download_url`; do not call final_output before upload succeeds, and do not use the local path, " "filename, URL, or a synthesized dify-file-ref as the reference)" ) if ( @@ -332,7 +332,7 @@ def _format_output_mention(output: DeclaredOutputConfig) -> str: f"{output.name} (array[file] output; upload each produced file with " f"`dify-agent file upload `, then set final_output.{output.name} to `tool_file` mappings " f"using the returned `reference` values; if replying to the user in natural language, use the returned " - f"`download_url`; do not call final_output before all uploads succeed, and do not use local paths, " + f"`public_download_url`; do not call final_output before all uploads succeed, and do not use local paths, " "filenames, URLs, or synthesized dify-file-ref values as references)" ) return f"{output.name} ({output.type.value})" diff --git a/api/services/agent_file_request_service.py b/api/services/agent_file_request_service.py deleted file mode 100644 index 091fc9fc59b..00000000000 --- a/api/services/agent_file_request_service.py +++ /dev/null @@ -1,93 +0,0 @@ -"""Resolve a download request for a workflow file ref to a signed URL (Agent Files §3.1.1/§4.5). - -The dify-agent server calls this on behalf of a sandbox that needs to pull a -``File`` / ``Array[File]`` workflow input. It binds the flattened file-access -context as a ``FileAccessScope``, rebuilds the graphon ``File`` from the mapping -(reusing tenant/user access checks), and returns an internal signed download URL -plus metadata — never the file bytes. The dify-agent server / sandbox then GETs -the URL directly from Dify API. -""" - -from __future__ import annotations - -from collections.abc import Mapping -from typing import Any - -from core.app.entities.app_invoke_entities import InvokeFrom, UserFrom -from core.app.file_access.controller import DatabaseFileAccessController -from core.app.file_access.scope import FileAccessScope, bind_file_access_scope -from core.app.workflow.file_runtime import DifyWorkflowFileRuntime -from factories import file_factory - - -class FileDownloadRequestError(Exception): - """A download-request failure mapped to an HTTP status by the controller.""" - - code: str - message: str - status_code: int - - def __init__(self, code: str, message: str, *, status_code: int = 400) -> None: - super().__init__(message) - self.code = code - self.message = message - self.status_code = status_code - - -class AgentFileDownloadRequestService: - """Resolve a workflow file ref to a sandbox-accessible internal signed download URL.""" - - @classmethod - def resolve( - cls, - *, - tenant_id: str, - user_id: str, - user_from: str, - invoke_from: str, - file_mapping: Mapping[str, Any], - ) -> dict[str, Any]: - try: - scope_user_from = UserFrom(user_from) - scope_invoke_from = InvokeFrom(invoke_from) - except ValueError as exc: - raise FileDownloadRequestError("invalid_access_context", str(exc), status_code=400) from exc - - if not isinstance(file_mapping, Mapping) or not file_mapping.get("transfer_method"): - raise FileDownloadRequestError("invalid_file_mapping", "file.transfer_method is required", status_code=400) - - scope = FileAccessScope( - tenant_id=tenant_id, - user_id=user_id, - user_from=scope_user_from, - invoke_from=scope_invoke_from, - ) - controller = DatabaseFileAccessController() - runtime = DifyWorkflowFileRuntime(file_access_controller=controller) - try: - with bind_file_access_scope(scope): - file = file_factory.build_from_mapping( - mapping=file_mapping, - tenant_id=tenant_id, - access_controller=controller, - ) - # Internal URL (for_external=False): the consumer is the agent backend / - # sandbox, not a browser. Resolves against INTERNAL_FILES_URL, falling - # back to FILES_URL when not configured. - download_url = runtime.resolve_file_url(file=file, for_external=False) - except ValueError as exc: - raise FileDownloadRequestError("file_not_accessible", str(exc), status_code=404) from exc - - if not download_url: - raise FileDownloadRequestError( - "download_url_unavailable", "could not resolve a download URL for the file", status_code=502 - ) - return { - "filename": file.filename, - "mime_type": file.mime_type, - "size": file.size, - "download_url": download_url, - } - - -__all__ = ["AgentFileDownloadRequestService", "FileDownloadRequestError"] diff --git a/api/services/file_request_service.py b/api/services/file_request_service.py index a795595d095..66b56684073 100644 --- a/api/services/file_request_service.py +++ b/api/services/file_request_service.py @@ -1,9 +1,9 @@ """Service helpers for trusted file request control-plane endpoints. -These helpers are used by inner APIs that return signed upload/download URLs to -trusted external runtimes such as ``dify-agent``. They do not transfer file -bytes themselves; they only rebuild access-scoped ``graphon.file.File`` values -and resolve the signed URL that the caller should use directly. +These helpers are used by inner APIs that allocate file access for trusted +external runtimes. They rebuild access-scoped ``graphon.file.File`` values and +return origin-free signed URIs so each transport adapter can select its own +network origin without signing the file twice. """ from __future__ import annotations @@ -14,30 +14,32 @@ from typing import Any from core.app.entities.app_invoke_entities import InvokeFrom, UserFrom from core.app.file_access import DatabaseFileAccessController, FileAccessScope, bind_file_access_scope +from core.app.workflow.file_runtime import DifyWorkflowFileRuntime from factories.file_factory.builders import build_from_mapping from graphon.file import File -from graphon.file import helpers as file_helpers @dataclass(frozen=True, slots=True) class DownloadFileRequestResult: - """Resolved metadata and signed URL returned to trusted download callers.""" + """Resolved metadata and signed URI returned to trusted download callers.""" filename: str mime_type: str | None size: int - download_url: str + download_uri: str class FileRequestService: - """Resolve signed download URLs for trusted external file consumers.""" + """Resolve signed download URIs for trusted external file consumers.""" _access_controller: DatabaseFileAccessController + _runtime: DifyWorkflowFileRuntime def __init__(self, access_controller: DatabaseFileAccessController | None = None) -> None: self._access_controller = access_controller or DatabaseFileAccessController() + self._runtime = DifyWorkflowFileRuntime(file_access_controller=self._access_controller) - def request_download_url( + def request_download( self, *, tenant_id: str, @@ -45,7 +47,6 @@ class FileRequestService: user_from: UserFrom | str, invoke_from: InvokeFrom | str, file_mapping: Mapping[str, Any], - for_external: bool = True, ) -> DownloadFileRequestResult: """Resolve one file mapping into signed download metadata. @@ -62,15 +63,15 @@ class FileRequestService: ) with bind_file_access_scope(scope): file = self._build_file(mapping=file_mapping, tenant_id=tenant_id) - download_url = file_helpers.resolve_file_url(file, for_external=for_external) + download_uri = self._runtime.resolve_file_uri(file=file) - if not download_url: + if not download_uri: raise ValueError("file does not support signed download") return DownloadFileRequestResult( filename=file.filename or "download.bin", mime_type=file.mime_type, size=file.size, - download_url=download_url, + download_uri=download_uri, ) def _build_file(self, *, mapping: Mapping[str, Any], tenant_id: str) -> File: diff --git a/api/tests/unit_tests/configs/test_dify_config.py b/api/tests/unit_tests/configs/test_dify_config.py index f8e6e890eef..ce81057f006 100644 --- a/api/tests/unit_tests/configs/test_dify_config.py +++ b/api/tests/unit_tests/configs/test_dify_config.py @@ -202,6 +202,16 @@ def test_internal_files_url_prefers_explicit_value(monkeypatch: pytest.MonkeyPat assert config.INTERNAL_FILES_URL == "http://files-internal:5001" +def test_empty_files_url_overrides_console_api_url_for_relative_browser_uris(monkeypatch: pytest.MonkeyPatch): + _clear_environment(monkeypatch) + monkeypatch.setenv("FILES_URL", "") + monkeypatch.setenv("CONSOLE_API_URL", "http://api:5001") + + config = DifyConfig(_env_file=None) + + assert config.FILES_URL == "" + + # NOTE: If there is a `.env` file in your Workspace, this test might not succeed as expected. # This is due to `pymilvus` loading all the variables from the `.env` file into `os.environ`. def test_flask_configs(monkeypatch: pytest.MonkeyPatch): diff --git a/api/tests/unit_tests/controllers/inner_api/plugin/test_plugin.py b/api/tests/unit_tests/controllers/inner_api/plugin/test_plugin.py index 249d8926124..c67e622ae51 100644 --- a/api/tests/unit_tests/controllers/inner_api/plugin/test_plugin.py +++ b/api/tests/unit_tests/controllers/inner_api/plugin/test_plugin.py @@ -263,11 +263,12 @@ class TestPluginUploadFileRequestApi: assert hasattr(api_instance, "post") assert callable(api_instance.post) - @patch("controllers.inner_api.plugin.plugin.get_signed_file_url_for_plugin") - def test_post_returns_signed_url(self, mock_get_url, api_instance, app: Flask): + @patch("controllers.inner_api.plugin.plugin.get_signed_file_uri_for_plugin") + def test_post_returns_signed_url(self, mock_get_uri, api_instance, app: Flask, monkeypatch: pytest.MonkeyPatch): """Test that post() generates a signed URL and returns it""" # Arrange - mock_get_url.return_value = "https://storage.example.com/signed-upload-url" + mock_get_uri.return_value = "/files/upload/for-plugin?sign=1" + monkeypatch.setattr(plugin_module.dify_config, "INTERNAL_FILES_URL", "http://api:5001") mock_tenant = MagicMock() mock_tenant.id = "tenant-id" mock_user = MagicMock() @@ -282,14 +283,14 @@ class TestPluginUploadFileRequestApi: result = raw_post(api_instance, user_model=mock_user, tenant_model=mock_tenant, payload=mock_payload) # Assert - mock_get_url.assert_called_once_with( + mock_get_uri.assert_called_once_with( filename="test.pdf", mimetype="application/pdf", tenant_id="tenant-id", user_id="user-id", conversation_id="conversation-id", ) - assert result["data"]["url"] == "https://storage.example.com/signed-upload-url" + assert result["data"]["url"] == "http://api:5001/files/upload/for-plugin?sign=1" class TestPluginDownloadFileRequestApi: @@ -304,6 +305,13 @@ class TestPluginDownloadFileRequestApi: assert callable(api_instance.post) @pytest.mark.parametrize("sqlite_session", [(Tenant,)], indirect=True) + @pytest.mark.parametrize( + ("for_external", "expected_url"), + [ + (True, "https://files.example.com/files/tools/report.pdf?sign=1"), + (False, "http://api:5001/files/tools/report.pdf?sign=1"), + ], + ) @patch("controllers.inner_api.plugin.plugin.FileRequestService") def test_post_returns_signed_download_url( self, @@ -312,6 +320,8 @@ class TestPluginDownloadFileRequestApi: app: Flask, monkeypatch: pytest.MonkeyPatch, sqlite_session: Session, + for_external: bool, + expected_url: str, ): tenant = Tenant( name="Plugin Tenant", @@ -324,18 +334,20 @@ class TestPluginDownloadFileRequestApi: sqlite_session.commit() monkeypatch.setattr(plugin_module.db, "session", sqlite_session) mock_service = mock_service_cls.return_value - mock_service.request_download_url.return_value = MagicMock( + mock_service.request_download.return_value = MagicMock( filename="report.pdf", mime_type="application/pdf", size=123, - download_url="https://files.example.com/download", + download_uri="/files/tools/report.pdf?sign=1", ) + monkeypatch.setattr(plugin_module.dify_config, "FILES_URL", "https://files.example.com") + monkeypatch.setattr(plugin_module.dify_config, "INTERNAL_FILES_URL", "http://api:5001") mock_payload = MagicMock() mock_payload.tenant_id = tenant.id mock_payload.user_id = "user-id" mock_payload.user_from = "account" mock_payload.invoke_from = "debugger" - mock_payload.for_external = False + mock_payload.for_external = for_external reference = build_file_reference(record_id="tool-file-1") mock_payload.file.model_dump.return_value = { "transfer_method": "tool_file", @@ -345,19 +357,18 @@ class TestPluginDownloadFileRequestApi: raw_post = _extract_raw_post(PluginDownloadFileRequestApi) result = raw_post(api_instance, payload=mock_payload) - mock_service.request_download_url.assert_called_once_with( + mock_service.request_download.assert_called_once_with( tenant_id=tenant.id, user_id="user-id", user_from="account", invoke_from="debugger", file_mapping={"transfer_method": "tool_file", "reference": reference}, - for_external=False, ) assert result["data"] == { "filename": "report.pdf", "mime_type": "application/pdf", "size": 123, - "download_url": "https://files.example.com/download", + "download_url": expected_url, } diff --git a/api/tests/unit_tests/controllers/inner_api/test_agent_files.py b/api/tests/unit_tests/controllers/inner_api/test_agent_files.py new file mode 100644 index 00000000000..d01f9b5512f --- /dev/null +++ b/api/tests/unit_tests/controllers/inner_api/test_agent_files.py @@ -0,0 +1,118 @@ +import inspect +from collections.abc import Callable +from types import SimpleNamespace +from typing import cast +from unittest.mock import MagicMock, patch + +import pytest +from flask import Flask + +from controllers.inner_api.agent.files import AgentFileDownloadRequestApi, AgentFileUploadRequestApi +from core.workflow.file_reference import build_file_reference +from services.file_request_service import DownloadFileRequestResult + +MODULE = "controllers.inner_api.agent.files" + + +def _raw[R](method: Callable[..., R]) -> Callable[..., R]: + return cast(Callable[..., R], inspect.unwrap(method)) + + +def test_upload_request_returns_origin_free_uri(app: Flask) -> None: + payload = { + "tenant_id": "tenant-1", + "user_id": "execution-user-1", + "filename": "report.pdf", + "mimetype": "application/pdf", + "conversation_id": "conversation-1", + } + tenant = SimpleNamespace(id="tenant-1") + user = SimpleNamespace(id="canonical-end-user-1") + session = MagicMock() + with app.test_request_context("/", method="POST", json=payload): + with ( + patch(f"{MODULE}.TenantService") as tenant_service, + patch(f"{MODULE}.get_user", return_value=user), + patch(f"{MODULE}.get_signed_file_uri_for_plugin", return_value="/files/upload/for-plugin?sign=1") as sign, + ): + tenant_service.get_tenant_by_id.return_value = tenant + response = _raw(AgentFileUploadRequestApi.post)(AgentFileUploadRequestApi(), session) + + assert response == {"upload_uri": "/files/upload/for-plugin?sign=1"} + tenant_service.get_tenant_by_id.assert_called_once_with("tenant-1", session=session) + sign.assert_called_once_with( + filename="report.pdf", + mimetype="application/pdf", + tenant_id="tenant-1", + user_id="canonical-end-user-1", + conversation_id="conversation-1", + ) + + +def test_download_request_returns_origin_free_uri_for_sandbox(app: Flask) -> None: + reference = build_file_reference(record_id="tool-file-1") + payload = { + "tenant_id": "tenant-1", + "user_id": "user-1", + "user_from": "account", + "invoke_from": "debugger", + "file": {"transfer_method": "tool_file", "reference": reference}, + "for_frontend": False, + } + session = MagicMock() + with app.test_request_context("/", method="POST", json=payload): + with ( + patch(f"{MODULE}.TenantService") as tenant_service, + patch(f"{MODULE}.FileRequestService") as service, + ): + tenant_service.get_tenant_by_id.return_value = MagicMock() + service.return_value.request_download.return_value = DownloadFileRequestResult( + filename="report.pdf", + mime_type="application/pdf", + size=123, + download_uri="/files/tools/tool-file-1.pdf?sign=1", + ) + response = _raw(AgentFileDownloadRequestApi.post)(AgentFileDownloadRequestApi(), session) + + assert response == { + "filename": "report.pdf", + "mime_type": "application/pdf", + "size": 123, + "download_uri": "/files/tools/tool-file-1.pdf?sign=1", + } + service.return_value.request_download.assert_called_once_with( + tenant_id="tenant-1", + user_id="user-1", + user_from="account", + invoke_from="debugger", + file_mapping={"transfer_method": "tool_file", "reference": reference}, + ) + + +def test_download_request_binds_frontend_url(app: Flask, monkeypatch: pytest.MonkeyPatch) -> None: + reference = build_file_reference(record_id="tool-file-1") + payload = { + "tenant_id": "tenant-1", + "user_id": "user-1", + "user_from": "account", + "invoke_from": "debugger", + "file": {"transfer_method": "tool_file", "reference": reference}, + "for_frontend": True, + } + monkeypatch.setattr(f"{MODULE}.dify_config.FILES_URL", "https://files.example.com") + session = MagicMock() + with app.test_request_context("/", method="POST", json=payload): + with ( + patch(f"{MODULE}.TenantService") as tenant_service, + patch(f"{MODULE}.FileRequestService") as service, + ): + tenant_service.get_tenant_by_id.return_value = MagicMock() + service.return_value.request_download.return_value = DownloadFileRequestResult( + filename="report.pdf", + mime_type="application/pdf", + size=123, + download_uri="/files/tools/tool-file-1.pdf?sign=1", + ) + response = _raw(AgentFileDownloadRequestApi.post)(AgentFileDownloadRequestApi(), session) + + assert response["download_uri"] == "https://files.example.com/files/tools/tool-file-1.pdf?sign=1" diff --git a/api/tests/unit_tests/core/app/workflow/test_file_runtime.py b/api/tests/unit_tests/core/app/workflow/test_file_runtime.py index 22b6f0c72bf..02b4ad362ec 100644 --- a/api/tests/unit_tests/core/app/workflow/test_file_runtime.py +++ b/api/tests/unit_tests/core/app/workflow/test_file_runtime.py @@ -141,8 +141,9 @@ def test_resolve_file_url_requires_extension_for_tool_files() -> None: def test_resolve_file_url_uses_tool_signatures_for_tool_and_datasource_files( monkeypatch: pytest.MonkeyPatch, ) -> None: - sign_tool_file = MagicMock(return_value="https://signed.example.com/file") - monkeypatch.setattr(file_runtime, "sign_tool_file", sign_tool_file) + sign_tool_file_uri = MagicMock(return_value="/files/signed") + monkeypatch.setattr(file_runtime, "sign_tool_file_uri", sign_tool_file_uri) + monkeypatch.setattr(file_runtime.dify_config, "FILES_URL", "https://files.example.com") runtime = _build_runtime() tool_file = _build_file( @@ -156,9 +157,35 @@ def test_resolve_file_url_uses_tool_signatures_for_tool_and_datasource_files( extension=".png", ) - assert runtime.resolve_file_url(file=tool_file) == "https://signed.example.com/file" - assert runtime.resolve_file_url(file=datasource_file) == "https://signed.example.com/file" - assert sign_tool_file.call_count == 2 + assert runtime.resolve_file_url(file=tool_file) == "https://files.example.com/files/signed" + assert runtime.resolve_file_url(file=datasource_file) == "https://files.example.com/files/signed" + assert sign_tool_file_uri.call_count == 2 + + +def test_resolve_file_uri_keeps_dify_owned_file_origin_free(monkeypatch: pytest.MonkeyPatch) -> None: + sign_tool_file_uri = MagicMock(return_value="/files/tools/tool-file-id.png?sign=1") + monkeypatch.setattr(file_runtime, "sign_tool_file_uri", sign_tool_file_uri) + runtime = _build_runtime() + file = _build_file( + transfer_method=FileTransferMethod.TOOL_FILE, + reference=build_file_reference(record_id="tool-file-id"), + extension=".png", + ) + + assert runtime.resolve_file_uri(file=file) == "/files/tools/tool-file-id.png?sign=1" + + +def test_resolve_file_url_returns_relative_uri_when_files_url_is_empty(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(file_runtime, "sign_tool_file_uri", lambda **_: "/files/tools/tool-file-id.png?sign=1") + monkeypatch.setattr(file_runtime.dify_config, "FILES_URL", "") + runtime = _build_runtime() + file = _build_file( + transfer_method=FileTransferMethod.TOOL_FILE, + reference=build_file_reference(record_id="tool-file-id"), + extension=".png", + ) + + assert runtime.resolve_file_url(file=file, for_external=True) == "/files/tools/tool-file-id.png?sign=1" def test_resolve_upload_file_url_signs_internal_urls_and_supports_attachments( diff --git a/api/tests/unit_tests/core/tools/test_signature.py b/api/tests/unit_tests/core/tools/test_signature.py index ee80cedd499..369c61455f8 100644 --- a/api/tests/unit_tests/core/tools/test_signature.py +++ b/api/tests/unit_tests/core/tools/test_signature.py @@ -7,14 +7,40 @@ from urllib.parse import parse_qs, urlparse import pytest from core.tools.signature import ( - get_signed_file_url_for_plugin, + bind_file_uri, + get_signed_file_uri_for_plugin, sign_tool_file, + sign_tool_file_uri, sign_upload_file_preview_url, verify_plugin_file_signature, verify_tool_file_signature, ) +def test_bind_file_uri_uses_selected_base_and_preserves_remote_url() -> None: + uri = "/files/tools/tool-file-id.png?sign=1" + + assert bind_file_uri(uri, "https://files.example.com") == f"https://files.example.com{uri}" + assert bind_file_uri(uri, "") == uri + assert bind_file_uri("https://remote.example.com/report.pdf", "https://files.example.com") == ( + "https://remote.example.com/report.pdf" + ) + + +def test_sign_tool_file_uri_has_no_origin(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr("core.tools.signature.time.time", lambda: 1700000000) + monkeypatch.setattr("core.tools.signature.os.urandom", lambda _: b"\x08" * 16) + monkeypatch.setattr("core.tools.signature.dify_config.SECRET_KEY", "unit-secret") + + uri = sign_tool_file_uri("tool-file-id", ".png") + parsed = urlparse(uri) + + assert parsed.scheme == "" + assert parsed.netloc == "" + assert parsed.path == "/files/tools/tool-file-id.png" + assert parse_qs(parsed.query)["timestamp"] == ["1700000000"] + + def test_sign_tool_file_and_verify_roundtrip(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr("core.tools.signature.time.time", lambda: 1700000000) monkeypatch.setattr("core.tools.signature.os.urandom", lambda _: b"\x01" * 16) @@ -125,25 +151,23 @@ def test_sign_upload_file_preview_url_ignores_internal_files_url(monkeypatch: py assert query["sign"][0] -def test_get_signed_file_url_for_plugin_and_verify_roundtrip(monkeypatch: pytest.MonkeyPatch) -> None: +def test_get_signed_file_uri_for_plugin_and_verify_roundtrip(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr("core.tools.signature.time.time", lambda: 1700000000) monkeypatch.setattr("core.tools.signature.os.urandom", lambda _: b"\x06" * 16) monkeypatch.setattr("core.tools.signature.dify_config.SECRET_KEY", "unit-secret") - monkeypatch.setattr("core.tools.signature.dify_config.FILES_URL", "https://files.example.com") - monkeypatch.setattr("core.tools.signature.dify_config.INTERNAL_FILES_URL", "https://internal.example.com") monkeypatch.setattr("core.tools.signature.dify_config.FILES_ACCESS_TIMEOUT", 60) - url = get_signed_file_url_for_plugin( + uri = get_signed_file_uri_for_plugin( filename="report.pdf", mimetype="application/pdf", tenant_id="tenant-id", user_id="user-id", conversation_id="conversation-id", ) - parsed = urlparse(url) + parsed = urlparse(uri) query = parse_qs(parsed.query) - assert parsed.netloc == "internal.example.com" + assert parsed.netloc == "" assert parsed.path == "/files/upload/for-plugin" assert query["tenant_id"] == ["tenant-id"] assert query["user_id"] == ["user-id"] @@ -167,17 +191,15 @@ def test_verify_plugin_file_signature_rejects_invalid_signatures(monkeypatch: py monkeypatch.setattr("core.tools.signature.time.time", lambda: 1700000000) monkeypatch.setattr("core.tools.signature.os.urandom", lambda _: b"\x07" * 16) monkeypatch.setattr("core.tools.signature.dify_config.SECRET_KEY", "unit-secret") - monkeypatch.setattr("core.tools.signature.dify_config.FILES_URL", "https://files.example.com") - monkeypatch.setattr("core.tools.signature.dify_config.INTERNAL_FILES_URL", "") monkeypatch.setattr("core.tools.signature.dify_config.FILES_ACCESS_TIMEOUT", 30) - url = get_signed_file_url_for_plugin( + uri = get_signed_file_uri_for_plugin( filename="report.pdf", mimetype="application/pdf", tenant_id="tenant-id", user_id="user-id", ) - query = parse_qs(urlparse(url).query) + query = parse_qs(urlparse(uri).query) assert ( verify_plugin_file_signature( diff --git a/api/tests/unit_tests/core/workflow/nodes/agent_v2/test_runtime_request_builder.py b/api/tests/unit_tests/core/workflow/nodes/agent_v2/test_runtime_request_builder.py index f52bc63bdc1..36a4e9a5ee9 100644 --- a/api/tests/unit_tests/core/workflow/nodes/agent_v2/test_runtime_request_builder.py +++ b/api/tests/unit_tests/core/workflow/nodes/agent_v2/test_runtime_request_builder.py @@ -432,7 +432,7 @@ def test_builds_workflow_run_request_with_file_output_schema_and_reserved_metada assert "never invent the `reference` value" in output_description assert "Do not call `final_output` before the upload command succeeds" in output_description assert "accepted file-mapping shape and the returned `reference`" in output_description - assert "include the returned `download_url` in that reply" in output_description + assert "include the returned `public_download_url` in that reply" in output_description assert output_schema["properties"]["confidence"]["type"] == "number" assert output_schema["required"] == ["report"] assert layers[DIFY_AGENT_MODEL_LAYER_ID]["config"]["model_settings"] == {"temperature": 0.2} diff --git a/api/tests/unit_tests/pyrefly.toml b/api/tests/unit_tests/pyrefly.toml index 4a2f4e0e0da..d5db248382f 100644 --- a/api/tests/unit_tests/pyrefly.toml +++ b/api/tests/unit_tests/pyrefly.toml @@ -820,7 +820,6 @@ project-excludes = [ "services/test_agent_app_sandbox_service.py", "services/test_agent_config_service.py", "services/test_agent_drive_service.py", - "services/test_agent_file_request_service.py", "services/test_annotation_service.py", "services/test_api_token_service.py", "services/test_app_generate_service.py", diff --git a/api/tests/unit_tests/services/agent/test_prompt_mentions.py b/api/tests/unit_tests/services/agent/test_prompt_mentions.py index 82c42a5bd86..48e4978a3bc 100644 --- a/api/tests/unit_tests/services/agent/test_prompt_mentions.py +++ b/api/tests/unit_tests/services/agent/test_prompt_mentions.py @@ -246,7 +246,7 @@ def test_node_job_resolver_resolves_each_kind(node_job: WorkflowNodeJobConfig): "Read START/tenders and produce qna_report (file output; create the file locally, run " "`dify-agent file upload `, then set final_output.qna_report to a `tool_file` mapping " "using the returned `reference`; if replying to the user in natural language, use the returned " - "`download_url`; do not call final_output before upload succeeds, and do not use the local path, " + "`public_download_url`; do not call final_output before upload succeeds, and do not use the local path, " "filename, URL, or a synthesized dify-file-ref as the reference); " "if unsure contact EMAIL · David Hayes." ) diff --git a/api/tests/unit_tests/services/test_agent_file_request_service.py b/api/tests/unit_tests/services/test_agent_file_request_service.py deleted file mode 100644 index c39e3f82558..00000000000 --- a/api/tests/unit_tests/services/test_agent_file_request_service.py +++ /dev/null @@ -1,105 +0,0 @@ -"""Unit tests for the Agent Files download-request service (ENG-592).""" - -from __future__ import annotations - -from types import SimpleNamespace -from unittest.mock import patch - -import pytest - -from services.agent_file_request_service import AgentFileDownloadRequestService, FileDownloadRequestError - -_MOD = "services.agent_file_request_service" - - -def _fake_file() -> SimpleNamespace: - return SimpleNamespace(filename="report.pdf", mime_type="application/pdf", size=12) - - -def test_resolve_returns_metadata_and_internal_url(): - with ( - patch(f"{_MOD}.file_factory.build_from_mapping", return_value=_fake_file()) as build, - patch(f"{_MOD}.DifyWorkflowFileRuntime") as runtime_cls, - ): - runtime_cls.return_value.resolve_file_url.return_value = "http://internal/files/x?sign=1" - data = AgentFileDownloadRequestService.resolve( - tenant_id="tenant-1", - user_id="user-1", - user_from="account", - invoke_from="service-api", - file_mapping={"transfer_method": "tool_file", "reference": "tool-file-1"}, - ) - - assert data == { - "filename": "report.pdf", - "mime_type": "application/pdf", - "size": 12, - "download_url": "http://internal/files/x?sign=1", - } - assert build.call_args.kwargs["tenant_id"] == "tenant-1" - # Sandbox/agent backend consumes the URL -> must be internal, not external. - assert runtime_cls.return_value.resolve_file_url.call_args.kwargs["for_external"] is False - - -@pytest.mark.parametrize( - ("user_from", "invoke_from", "code"), - [ - ("bogus", "service-api", "invalid_access_context"), - ("account", "not-a-source", "invalid_access_context"), - ], -) -def test_invalid_access_context_rejected(user_from: str, invoke_from: str, code: str): - with pytest.raises(FileDownloadRequestError) as exc_info: - AgentFileDownloadRequestService.resolve( - tenant_id="t", - user_id="u", - user_from=user_from, - invoke_from=invoke_from, - file_mapping={"transfer_method": "tool_file", "reference": "x"}, - ) - assert exc_info.value.status_code == 400 - assert exc_info.value.code == code - - -def test_missing_transfer_method_rejected(): - with pytest.raises(FileDownloadRequestError) as exc_info: - AgentFileDownloadRequestService.resolve( - tenant_id="t", - user_id="u", - user_from="account", - invoke_from="service-api", - file_mapping={}, - ) - assert exc_info.value.status_code == 400 - assert exc_info.value.code == "invalid_file_mapping" - - -def test_inaccessible_file_maps_to_404(): - with patch(f"{_MOD}.file_factory.build_from_mapping", side_effect=ValueError("ToolFile x not found")): - with pytest.raises(FileDownloadRequestError) as exc_info: - AgentFileDownloadRequestService.resolve( - tenant_id="t", - user_id="u", - user_from="end-user", - invoke_from="web-app", - file_mapping={"transfer_method": "tool_file", "reference": "x"}, - ) - assert exc_info.value.status_code == 404 - assert exc_info.value.code == "file_not_accessible" - - -def test_unresolved_url_maps_to_502(): - with ( - patch(f"{_MOD}.file_factory.build_from_mapping", return_value=_fake_file()), - patch(f"{_MOD}.DifyWorkflowFileRuntime") as runtime_cls, - ): - runtime_cls.return_value.resolve_file_url.return_value = None - with pytest.raises(FileDownloadRequestError) as exc_info: - AgentFileDownloadRequestService.resolve( - tenant_id="t", - user_id="u", - user_from="account", - invoke_from="service-api", - file_mapping={"transfer_method": "tool_file", "reference": "x"}, - ) - assert exc_info.value.status_code == 502 diff --git a/api/tests/unit_tests/services/test_file_request_service.py b/api/tests/unit_tests/services/test_file_request_service.py index 3e2ea0a258a..9acfa38a026 100644 --- a/api/tests/unit_tests/services/test_file_request_service.py +++ b/api/tests/unit_tests/services/test_file_request_service.py @@ -15,7 +15,7 @@ from services.file_request_service import FileRequestService ("end-user", "service-api", UserFrom.END_USER, InvokeFrom.SERVICE_API), ], ) -def test_request_download_url_builds_file_under_bound_scope( +def test_request_download_builds_file_under_bound_scope( user_from: UserFrom | str, invoke_from: InvokeFrom | str, expected_user_from: UserFrom, @@ -29,12 +29,9 @@ def test_request_download_url_builds_file_under_bound_scope( with ( patch("services.file_request_service.bind_file_access_scope", return_value=nullcontext()) as bind_scope, patch.object(service, "_build_file", return_value=fake_file) as build_file, - patch( - "services.file_request_service.file_helpers.resolve_file_url", - return_value="https://files.example.com/x", - ) as resolve_file_url, + patch.object(service._runtime, "resolve_file_uri", return_value="/files/tools/x?sign=1") as resolve_file_uri, ): - result = service.request_download_url( + result = service.request_download( tenant_id="tenant-1", user_id="user-1", user_from=user_from, @@ -52,48 +49,23 @@ def test_request_download_url_builds_file_under_bound_scope( build_file.assert_called_once_with( mapping={"transfer_method": "tool_file", "reference": reference}, tenant_id="tenant-1" ) - resolve_file_url.assert_called_once_with(fake_file, for_external=True) + resolve_file_uri.assert_called_once_with(file=fake_file) assert result.filename == "report.pdf" assert result.mime_type == "application/pdf" assert result.size == 123 - assert result.download_url == "https://files.example.com/x" + assert result.download_uri == "/files/tools/x?sign=1" -def test_request_download_url_supports_internal_download_urls() -> None: - fake_file = MagicMock(filename="report.pdf", mime_type="application/pdf", size=123) - service = FileRequestService(access_controller=MagicMock()) - - with ( - patch("services.file_request_service.bind_file_access_scope", return_value=nullcontext()), - patch.object(service, "_build_file", return_value=fake_file), - patch( - "services.file_request_service.file_helpers.resolve_file_url", - return_value="http://internal-files/report.pdf", - ) as resolve_file_url, - ): - result = service.request_download_url( - tenant_id="tenant-1", - user_id="user-1", - user_from="account", - invoke_from="debugger", - file_mapping={"transfer_method": "tool_file", "reference": "dify-file-ref:tool-file-1"}, - for_external=False, - ) - - resolve_file_url.assert_called_once_with(fake_file, for_external=False) - assert result.download_url == "http://internal-files/report.pdf" - - -def test_request_download_url_rejects_unsupported_files() -> None: +def test_request_download_rejects_unsupported_files() -> None: service = FileRequestService(access_controller=MagicMock()) with ( patch("services.file_request_service.bind_file_access_scope", return_value=nullcontext()), patch.object(service, "_build_file", return_value=MagicMock(filename="report.pdf", mime_type=None, size=1)), - patch("services.file_request_service.file_helpers.resolve_file_url", return_value=None), + patch.object(service._runtime, "resolve_file_uri", return_value=None), ): with pytest.raises(ValueError, match="file does not support signed download"): - service.request_download_url( + service.request_download( tenant_id="tenant-1", user_id="user-1", user_from="account", diff --git a/dify-agent-runtime/gen/dify/agent/stub/v1/agent_stub.pb.go b/dify-agent-runtime/gen/dify/agent/stub/v1/agent_stub.pb.go index 4cfc11903d8..1d67854634a 100644 --- a/dify-agent-runtime/gen/dify/agent/stub/v1/agent_stub.pb.go +++ b/dify-agent-runtime/gen/dify/agent/stub/v1/agent_stub.pb.go @@ -292,7 +292,7 @@ func (x *FileMapping) GetUrl() string { type FileDownloadRequest struct { state protoimpl.MessageState `protogen:"open.v1"` File *FileMapping `protobuf:"bytes,1,opt,name=file,proto3" json:"file,omitempty"` - ForExternal *bool `protobuf:"varint,2,opt,name=for_external,json=forExternal,proto3,oneof" json:"for_external,omitempty"` + ForFrontend *bool `protobuf:"varint,2,opt,name=for_frontend,json=forFrontend,proto3,oneof" json:"for_frontend,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -334,9 +334,9 @@ func (x *FileDownloadRequest) GetFile() *FileMapping { return nil } -func (x *FileDownloadRequest) GetForExternal() bool { - if x != nil && x.ForExternal != nil { - return *x.ForExternal +func (x *FileDownloadRequest) GetForFrontend() bool { + if x != nil && x.ForFrontend != nil { + return *x.ForFrontend } return false } @@ -436,8 +436,8 @@ const file_dify_agent_stub_v1_agent_stub_proto_rawDesc = "" + "\x04_url\"\x83\x01\n" + "\x13FileDownloadRequest\x123\n" + "\x04file\x18\x01 \x01(\v2\x1f.dify.agent.stub.v1.FileMappingR\x04file\x12&\n" + - "\ffor_external\x18\x02 \x01(\bH\x00R\vforExternal\x88\x01\x01B\x0f\n" + - "\r_for_external\"\x99\x01\n" + + "\ffor_frontend\x18\x02 \x01(\bH\x00R\vforFrontend\x88\x01\x01B\x0f\n" + + "\r_for_frontend\"\x99\x01\n" + "\x14FileDownloadResponse\x12\x1a\n" + "\bfilename\x18\x01 \x01(\tR\bfilename\x12 \n" + "\tmime_type\x18\x02 \x01(\tH\x00R\bmimeType\x88\x01\x01\x12\x12\n" + diff --git a/dify-agent-runtime/internal/agentcli/client.go b/dify-agent-runtime/internal/agentcli/client.go index 328386cdac1..ecc5feb94d9 100644 --- a/dify-agent-runtime/internal/agentcli/client.go +++ b/dify-agent-runtime/internal/agentcli/client.go @@ -8,7 +8,7 @@ type StubClient interface { // Control-plane: available via gRPC or HTTP Connect(ctx context.Context, argv []string, metadataJSON string) (*ConnectResponse, error) CreateFileUploadURL(ctx context.Context, filename, mimetype string) (string, error) - CreateFileDownloadURL(ctx context.Context, transferMethod string, reference, url *string, forExternal bool) (*FileDownloadResponse, error) + CreateFileDownloadURL(ctx context.Context, transferMethod string, reference, url *string, forFrontend bool) (*FileDownloadResponse, error) // Drive operations (HTTP-only control-plane) GetDriveManifest(ctx context.Context, prefix string, includeDownloadURL bool) (*DriveManifestResponse, error) diff --git a/dify-agent-runtime/internal/agentcli/client_grpc.go b/dify-agent-runtime/internal/agentcli/client_grpc.go index 6a5f14b715d..ba6764b9d98 100644 --- a/dify-agent-runtime/internal/agentcli/client_grpc.go +++ b/dify-agent-runtime/internal/agentcli/client_grpc.go @@ -55,8 +55,8 @@ func (c *grpcStubClient) CreateFileUploadURL(ctx context.Context, filename, mime return result.UploadURL, nil } -func (c *grpcStubClient) CreateFileDownloadURL(ctx context.Context, transferMethod string, reference, url *string, forExternal bool) (*FileDownloadResponse, error) { - result, err := c.grpc.CreateFileDownload(ctx, transferMethod, reference, url, forExternal) +func (c *grpcStubClient) CreateFileDownloadURL(ctx context.Context, transferMethod string, reference, url *string, forFrontend bool) (*FileDownloadResponse, error) { + result, err := c.grpc.CreateFileDownload(ctx, transferMethod, reference, url, forFrontend) if err != nil { return nil, err } diff --git a/dify-agent-runtime/internal/agentcli/client_http.go b/dify-agent-runtime/internal/agentcli/client_http.go index 2a6031a54fd..c69c89b091a 100644 --- a/dify-agent-runtime/internal/agentcli/client_http.go +++ b/dify-agent-runtime/internal/agentcli/client_http.go @@ -72,7 +72,7 @@ func (c *httpStubClient) CreateFileUploadURL(_ context.Context, filename, mimety return resp.UploadURL, nil } -func (c *httpStubClient) CreateFileDownloadURL(_ context.Context, transferMethod string, reference, url *string, forExternal bool) (*FileDownloadResponse, error) { +func (c *httpStubClient) CreateFileDownloadURL(_ context.Context, transferMethod string, reference, url *string, forFrontend bool) (*FileDownloadResponse, error) { fileMapping := map[string]any{ "transfer_method": transferMethod, } @@ -85,7 +85,7 @@ func (c *httpStubClient) CreateFileDownloadURL(_ context.Context, transferMethod payload := map[string]any{ "file": fileMapping, - "for_external": forExternal, + "for_frontend": forFrontend, } body, statusCode, err := c.http.postJSON("/files/download-request", payload) if err != nil { diff --git a/dify-agent-runtime/internal/agentcli/file.go b/dify-agent-runtime/internal/agentcli/file.go index d0f8ab75f1e..93dd3aabe83 100644 --- a/dify-agent-runtime/internal/agentcli/file.go +++ b/dify-agent-runtime/internal/agentcli/file.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "io" "mime" "os" "path/filepath" @@ -12,9 +13,9 @@ import ( // FileUploadResponse is the JSON output for `dify-agent file upload`. type FileUploadResponse struct { - TransferMethod string `json:"transfer_method"` - Reference string `json:"reference"` - DownloadURL string `json:"download_url"` + TransferMethod string `json:"transfer_method"` + Reference string `json:"reference"` + PublicDownloadURL string `json:"public_download_url"` } // FileDownloadResponse is the response from a file download request. @@ -27,6 +28,27 @@ type FileDownloadResponse struct { // RunFileUpload executes the `file upload` command. func RunFileUpload(env *Environment, path string) error { + client, err := NewStubClient(env) + if err != nil { + return err + } + defer func() { _ = client.Close() }() + + return runFileUpload(client, path, os.Stdout) +} + +type fileUploadClient interface { + CreateFileUploadURL(ctx context.Context, filename, mimetype string) (string, error) + UploadFileToURL(uploadURL, filePath, filename, mimetype string) ([]byte, error) + CreateFileDownloadURL( + ctx context.Context, + transferMethod string, + reference, url *string, + forFrontend bool, + ) (*FileDownloadResponse, error) +} + +func runFileUpload(client fileUploadClient, path string, output io.Writer) error { absPath, err := filepath.Abs(path) if err != nil { return fmt.Errorf("resolve path: %w", err) @@ -40,12 +62,6 @@ func RunFileUpload(env *Environment, path string) error { mimetype := guessMIMEType(filename) ctx := context.Background() - client, err := NewStubClient(env) - if err != nil { - return err - } - defer func() { _ = client.Close() }() - // Step 1: Request a signed upload URL uploadURL, err := client.CreateFileUploadURL(ctx, filename, mimetype) if err != nil { @@ -70,18 +86,18 @@ func RunFileUpload(env *Environment, path string) error { // Step 3: Request download URL for the uploaded file ref := reference - dlResp, err := client.CreateFileDownloadURL(ctx, "tool_file", &ref, nil, false) + dlResp, err := client.CreateFileDownloadURL(ctx, "tool_file", &ref, nil, true) if err != nil { return err } result := FileUploadResponse{ - TransferMethod: "tool_file", - Reference: reference, - DownloadURL: dlResp.DownloadURL, + TransferMethod: "tool_file", + Reference: reference, + PublicDownloadURL: dlResp.DownloadURL, } out, _ := json.Marshal(result) - fmt.Println(string(out)) + _, _ = fmt.Fprintln(output, string(out)) return nil } diff --git a/dify-agent-runtime/internal/agentcli/file_test.go b/dify-agent-runtime/internal/agentcli/file_test.go new file mode 100644 index 00000000000..a76fcffb7f5 --- /dev/null +++ b/dify-agent-runtime/internal/agentcli/file_test.go @@ -0,0 +1,110 @@ +package agentcli + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +type fakeFileUploadClient struct { + forFrontend bool +} + +func (f *fakeFileUploadClient) CreateFileUploadURL(_ context.Context, filename, mimetype string) (string, error) { + return "https://sandbox-files.example.com/files/upload/for-plugin?sign=1", nil +} + +func (f *fakeFileUploadClient) UploadFileToURL(uploadURL, filePath, filename, mimetype string) ([]byte, error) { + return []byte(`{"reference":"dify-file-ref:canonical"}`), nil +} + +func (f *fakeFileUploadClient) CreateFileDownloadURL( + _ context.Context, + _ string, + _, _ *string, + forFrontend bool, +) (*FileDownloadResponse, error) { + f.forFrontend = forFrontend + return &FileDownloadResponse{ + Filename: "report.pdf", + MimeType: "application/pdf", + Size: 123, + DownloadURL: "/files/tools/report.pdf?sign=2", + }, nil +} + +func TestRunFileUploadReturnsFrontendDisplayURL(t *testing.T) { + filePath := t.TempDir() + "/report.pdf" + if err := os.WriteFile(filePath, []byte("report"), 0o600); err != nil { + t.Fatalf("write fixture: %v", err) + } + + client := &fakeFileUploadClient{} + var output bytes.Buffer + if err := runFileUpload(client, filePath, &output); err != nil { + t.Fatalf("run file upload: %v", err) + } + + if !client.forFrontend { + t.Fatal("download request did not select frontend display URL") + } + got := strings.TrimSpace(output.String()) + want := `{"transfer_method":"tool_file","reference":"dify-file-ref:canonical","public_download_url":"/files/tools/report.pdf?sign=2"}` + if got != want { + t.Fatalf("output = %s, want %s", got, want) + } +} + +func TestRunFileDownloadRequestsSandboxURLAndWritesFile(t *testing.T) { + var requestPayload map[string]json.RawMessage + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/agent-stub/files/download-request": + if err := json.NewDecoder(r.Body).Decode(&requestPayload); err != nil { + t.Errorf("decode download request: %v", err) + http.Error(w, "bad request", http.StatusBadRequest) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"filename":"report.pdf","mime_type":"application/pdf","size":6,"download_url":"` + server.URL + `/files/report.pdf"}`)) + case "/files/report.pdf": + _, _ = w.Write([]byte("report")) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + + targetDir := t.TempDir() + err := RunFileDownload( + &Environment{URL: server.URL + "/agent-stub", AuthJWE: "test-token"}, + "tool_file", + "dify-file-ref:canonical", + targetDir, + ) + if err != nil { + t.Fatalf("run file download: %v", err) + } + + var forFrontend bool + if err := json.Unmarshal(requestPayload["for_frontend"], &forFrontend); err != nil { + t.Fatalf("decode for_frontend: %v", err) + } + if forFrontend { + t.Fatal("download request selected a frontend URL") + } + data, err := os.ReadFile(filepath.Join(targetDir, "report.pdf")) + if err != nil { + t.Fatalf("read downloaded file: %v", err) + } + if string(data) != "report" { + t.Fatalf("downloaded file = %q, want report", data) + } +} diff --git a/dify-agent-runtime/internal/stubclient/client.go b/dify-agent-runtime/internal/stubclient/client.go index d9b6bdddc8a..9c3c0a659b1 100644 --- a/dify-agent-runtime/internal/stubclient/client.go +++ b/dify-agent-runtime/internal/stubclient/client.go @@ -113,7 +113,7 @@ type FileDownloadResult struct { } // CreateFileDownload requests a download URL from the Agent Stub server. -func (c *Client) CreateFileDownload(ctx context.Context, transferMethod string, reference, url *string, forExternal bool) (*FileDownloadResult, error) { +func (c *Client) CreateFileDownload(ctx context.Context, transferMethod string, reference, url *string, forFrontend bool) (*FileDownloadResult, error) { ctx, cancel := context.WithTimeout(ctx, c.timeout) defer cancel() @@ -129,7 +129,7 @@ func (c *Client) CreateFileDownload(ctx context.Context, transferMethod string, resp, err := c.stub.CreateFileDownloadRequest(ctx, &stubv1.FileDownloadRequest{ File: fileMapping, - ForExternal: &forExternal, + ForFrontend: &forFrontend, }) if err != nil { return nil, fmt.Errorf("stubclient: file download: %w", err) diff --git a/dify-agent/docs/dify-agent/get-started/index.md b/dify-agent/docs/dify-agent/get-started/index.md index bcad90cf144..b672c5fdb41 100644 --- a/dify-agent/docs/dify-agent/get-started/index.md +++ b/dify-agent/docs/dify-agent/get-started/index.md @@ -99,7 +99,8 @@ the request's product context; `DifyRuntimeLayerConfig.backend_binding_ref` carries only that opaque ref and opens a new operation-scoped `RuntimeLease` for the run. When shell jobs need to call back with the `dify-agent` command, also set -`DIFY_AGENT_STUB_API_BASE_URL`. The supplied default configs include a +`DIFY_AGENT_STUB_API_BASE_URL` and the Sandbox-reachable Dify API base +`DIFY_AGENT_SANDBOX_FILES_BASE_URL`. The supplied default configs include a development `DIFY_AGENT_SERVER_SECRET_KEY`, but production deployments should override it with a unique 32-byte base64url value as documented in `.example.env`. diff --git a/dify-agent/docs/dify-agent/guide/index.md b/dify-agent/docs/dify-agent/guide/index.md index 129568a5643..3ff80e1e28f 100644 --- a/dify-agent/docs/dify-agent/guide/index.md +++ b/dify-agent/docs/dify-agent/guide/index.md @@ -56,7 +56,8 @@ also reads `.env` and `dify-agent/.env` when present. | `DIFY_AGENT_E2B_SHELLCTL_PORT` | `5004` | shellctl port exposed by the E2B template. | | `DIFY_AGENT_SANDBOX_FILE_UPLOAD_MAX_BYTES` | `52428800` | Standalone Dify Agent maximum for whole-file Workspace upload capture; 50 MiB by default. Docker Compose derives it from `PLUGIN_MAX_FILE_SIZE`. | | `DIFY_AGENT_SHELL_REDACT_PATTERNS` | empty | JSON array of additional regex patterns redacted from Shell output. | -| `DIFY_AGENT_STUB_API_BASE_URL` | empty | Public Agent Stub API base URL reachable from shellctl-managed remote machines. HTTP may be the service root or `/agent-stub`; gRPC must be `grpc://host:port`. Enables `DIFY_AGENT_STUB_*` env injection for user `shell.run` jobs. | +| `DIFY_AGENT_STUB_API_BASE_URL` | empty | Agent Stub API base URL reachable from the Sandbox. HTTP may be the service root or `/agent-stub`; gRPC must be `grpc://host:port`. Enables `DIFY_AGENT_STUB_*` env injection for user `shell.run` jobs. | +| `DIFY_AGENT_SANDBOX_FILES_BASE_URL` | empty | Dify API base URL reachable from the Sandbox for signed `/files/*` upload/download bytes. Required when Agent Stub file operations are enabled. May include an ingress path prefix, but not a query or fragment. | | `DIFY_AGENT_STUB_GRPC_BIND_ADDRESS` | empty | Optional `host:port` bind override used only when `DIFY_AGENT_STUB_API_BASE_URL` uses `grpc://`. | | `DIFY_AGENT_SERVER_SECRET_KEY` | empty | Security-sensitive server-wide root secret used to derive the JWE encryption key for Agent Stub bearer tokens; required when `DIFY_AGENT_STUB_API_BASE_URL` is set. The supplied default config uses a development value; set a unique unpadded base64url 32-byte secret in production. | | `DIFY_AGENT_OUTBOUND_HTTP_CONNECT_TIMEOUT` | `10` | Shared outbound HTTP connect timeout in seconds. | @@ -87,12 +88,30 @@ DIFY_AGENT_LOCAL_SANDBOX_WORKSPACE_ROOT=/tmp/dify-agent/workspaces DIFY_AGENT_LOCAL_SANDBOX_HOME_SNAPSHOT_ROOT=/tmp/dify-agent/home-snapshots DIFY_AGENT_SANDBOX_FILE_UPLOAD_MAX_BYTES=52428800 DIFY_AGENT_STUB_API_BASE_URL=https://agent.example.com/agent-stub +DIFY_AGENT_SANDBOX_FILES_BASE_URL=https://dify.example.com # This is security-sensitive: it derives the JWE encryption key for Agent Stub bearer tokens. # Replace this development default in production. # Generate one with: python -c 'import secrets; print(secrets.token_urlsafe(32))' DIFY_AGENT_SERVER_SECRET_KEY=MDEyMzQ1Njc4OWFiY2RlZjAxMjM0NTY3ODlhYmNkZWY ``` +The two Sandbox-facing base URLs have different owners. Agent Stub control +requests use `DIFY_AGENT_STUB_API_BASE_URL`; signed file bytes use +`DIFY_AGENT_SANDBOX_FILES_BASE_URL`. `DIFY_AGENT_INNER_API_URL` remains a +trusted service-to-service URL and is never returned to the Sandbox. + +For a remote Sandbox, expose only `/agent-stub/*` from Agent Backend and the +existing `/files/*` Dify API data plane. The `/files/*` ingress must preserve +the complete signed query string, allow the configured upload body size, and +use response streaming and timeouts suitable for large downloads. Do not expose +Agent Backend `/runs`, Workspace, or Binding management routes through the +Sandbox ingress. + +Browser presentation URLs are independent. Configure Dify API `FILES_URL` to a +browser-reachable public origin, or leave it empty so responses use same-origin +relative `/files/...` URIs. Never set `FILES_URL` to a Docker-only service name +such as `http://api:5001`. + `DIFY_AGENT_SHELLCTL_ENTRYPOINT` and `DIFY_AGENT_SHELLCTL_AUTH_TOKEN` remain accepted only as legacy aliases for the two Local settings. New deployments must use `DIFY_AGENT_LOCAL_SANDBOX_ENDPOINT` and diff --git a/dify-agent/docs/dify-agent/user-manual/shell-layer/index.md b/dify-agent/docs/dify-agent/user-manual/shell-layer/index.md index 188add6cfc3..0f1d923ffea 100644 --- a/dify-agent/docs/dify-agent/user-manual/shell-layer/index.md +++ b/dify-agent/docs/dify-agent/user-manual/shell-layer/index.md @@ -80,16 +80,40 @@ DIFY_AGENT_LOCAL_SANDBOX_AUTH_TOKEN=replace-with-shellctl-token The auth token may be empty when shellctl authentication is disabled. E2B uses `DIFY_AGENT_E2B_API_KEY`, the prepared template, and its shellctl settings. -To let shell jobs call the Agent Stub with `dify-agent ...`, configure a public -Agent Stub URL and a unique production secret: +To let shell jobs call the Agent Stub with `dify-agent ...`, configure a +Sandbox-reachable Agent Stub URL and a unique production secret. Remote +deployments normally use a public Agent ingress. Local Compose uses +`http://agent_backend:5050/agent-stub`, reached through the existing +`agent_ssrf_proxy`; this configuration does not change the Compose network +topology. ```env DIFY_AGENT_STUB_API_BASE_URL=https://agent.example.com/agent-stub +DIFY_AGENT_SANDBOX_FILES_BASE_URL=https://dify.example.com DIFY_AGENT_SERVER_SECRET_KEY=replace-with-unpadded-base64url-for-32-random-bytes ``` HTTP URLs may be either the service root or the explicit `/agent-stub` root. -The server normalizes a service root and rejects unrelated paths. +The server normalizes a service root and rejects unrelated paths. The separate +Sandbox file base must point to the Dify API ingress serving `/files/*`; it is +used for CLI upload/download bytes even when Agent Stub control calls use gRPC. + +After `dify-agent file upload ` succeeds, the CLI prints JSON such as: + +```json +{ + "transfer_method": "tool_file", + "reference": "dify-file-ref:...", + "public_download_url": "https://dify.example.com/files/tools/..." +} +``` + +`reference` is the persistent canonical file identity and should be stored in +structured output. `public_download_url` is a short-lived frontend presentation +address: it is an absolute URL when Dify API `FILES_URL` has a public origin, +or a same-origin `/files/...` relative URI when `FILES_URL` is empty. The CLI +does not access this field. The former ambiguous `download_url` upload-output +key is now named `public_download_url`. ## Request graph diff --git a/dify-agent/proto/dify/agent/stub/v1/agent_stub.proto b/dify-agent/proto/dify/agent/stub/v1/agent_stub.proto index 608d6d0006c..fddd165feba 100644 --- a/dify-agent/proto/dify/agent/stub/v1/agent_stub.proto +++ b/dify-agent/proto/dify/agent/stub/v1/agent_stub.proto @@ -38,7 +38,7 @@ message FileMapping { message FileDownloadRequest { FileMapping file = 1; - optional bool for_external = 2; + optional bool for_frontend = 2; } message FileDownloadResponse { diff --git a/dify-agent/src/dify_agent/agent_stub/grpc/_generated/agent_stub_pb2.py b/dify-agent/src/dify_agent/agent_stub/grpc/_generated/agent_stub_pb2.py index db940989607..3b43456bf94 100644 --- a/dify-agent/src/dify_agent/agent_stub/grpc/_generated/agent_stub_pb2.py +++ b/dify-agent/src/dify_agent/agent_stub/grpc/_generated/agent_stub_pb2.py @@ -20,14 +20,15 @@ _sym_db = _symbol_database.Default() DESCRIPTOR = _descriptor_pool.Default().AddSerializedFile( - b'\n#dify/agent/stub/v1/agent_stub.proto\x12\x12\x64ify.agent.stub.v1"O\n\x0e\x43onnectRequest\x12\x18\n\x10protocol_version\x18\x01 \x01(\x05\x12\x0c\n\x04\x61rgv\x18\x02 \x03(\t\x12\x15\n\rmetadata_json\x18\x03 \x01(\t"8\n\x0f\x43onnectResponse\x12\x15\n\rconnection_id\x18\x01 \x01(\t\x12\x0e\n\x06status\x18\x02 \x01(\t"7\n\x11\x46ileUploadRequest\x12\x10\n\x08\x66ilename\x18\x01 \x01(\t\x12\x10\n\x08mimetype\x18\x02 \x01(\t"(\n\x12\x46ileUploadResponse\x12\x12\n\nupload_url\x18\x01 \x01(\t"f\n\x0b\x46ileMapping\x12\x17\n\x0ftransfer_method\x18\x01 \x01(\t\x12\x16\n\treference\x18\x02 \x01(\tH\x00\x88\x01\x01\x12\x10\n\x03url\x18\x03 \x01(\tH\x01\x88\x01\x01\x42\x0c\n\n_referenceB\x06\n\x04_url"p\n\x13\x46ileDownloadRequest\x12-\n\x04\x66ile\x18\x01 \x01(\x0b\x32\x1f.dify.agent.stub.v1.FileMapping\x12\x19\n\x0c\x66or_external\x18\x02 \x01(\x08H\x00\x88\x01\x01\x42\x0f\n\r_for_external"r\n\x14\x46ileDownloadResponse\x12\x10\n\x08\x66ilename\x18\x01 \x01(\t\x12\x16\n\tmime_type\x18\x02 \x01(\tH\x00\x88\x01\x01\x12\x0c\n\x04size\x18\x03 \x01(\x03\x12\x14\n\x0c\x64ownload_url\x18\x04 \x01(\tB\x0c\n\n_mime_type2\xc0\x02\n\x10\x41gentStubService\x12R\n\x07\x43onnect\x12".dify.agent.stub.v1.ConnectRequest\x1a#.dify.agent.stub.v1.ConnectResponse\x12h\n\x17\x43reateFileUploadRequest\x12%.dify.agent.stub.v1.FileUploadRequest\x1a&.dify.agent.stub.v1.FileUploadResponse\x12n\n\x19\x43reateFileDownloadRequest\x12\'.dify.agent.stub.v1.FileDownloadRequest\x1a(.dify.agent.stub.v1.FileDownloadResponseb\x06proto3' + b'\n#dify/agent/stub/v1/agent_stub.proto\x12\x12\x64ify.agent.stub.v1"O\n\x0e\x43onnectRequest\x12\x18\n\x10protocol_version\x18\x01 \x01(\x05\x12\x0c\n\x04\x61rgv\x18\x02 \x03(\t\x12\x15\n\rmetadata_json\x18\x03 \x01(\t"8\n\x0f\x43onnectResponse\x12\x15\n\rconnection_id\x18\x01 \x01(\t\x12\x0e\n\x06status\x18\x02 \x01(\t"7\n\x11\x46ileUploadRequest\x12\x10\n\x08\x66ilename\x18\x01 \x01(\t\x12\x10\n\x08mimetype\x18\x02 \x01(\t"(\n\x12\x46ileUploadResponse\x12\x12\n\nupload_url\x18\x01 \x01(\t"f\n\x0b\x46ileMapping\x12\x17\n\x0ftransfer_method\x18\x01 \x01(\t\x12\x16\n\treference\x18\x02 \x01(\tH\x00\x88\x01\x01\x12\x10\n\x03url\x18\x03 \x01(\tH\x01\x88\x01\x01\x42\x0c\n\n_referenceB\x06\n\x04_url"p\n\x13\x46ileDownloadRequest\x12-\n\x04\x66ile\x18\x01 \x01(\x0b\x32\x1f.dify.agent.stub.v1.FileMapping\x12\x19\n\x0c\x66or_frontend\x18\x02 \x01(\x08H\x00\x88\x01\x01\x42\x0f\n\r_for_frontend"r\n\x14\x46ileDownloadResponse\x12\x10\n\x08\x66ilename\x18\x01 \x01(\t\x12\x16\n\tmime_type\x18\x02 \x01(\tH\x00\x88\x01\x01\x12\x0c\n\x04size\x18\x03 \x01(\x03\x12\x14\n\x0c\x64ownload_url\x18\x04 \x01(\tB\x0c\n\n_mime_type2\xc0\x02\n\x10\x41gentStubService\x12R\n\x07\x43onnect\x12".dify.agent.stub.v1.ConnectRequest\x1a#.dify.agent.stub.v1.ConnectResponse\x12h\n\x17\x43reateFileUploadRequest\x12%.dify.agent.stub.v1.FileUploadRequest\x1a&.dify.agent.stub.v1.FileUploadResponse\x12n\n\x19\x43reateFileDownloadRequest\x12\'.dify.agent.stub.v1.FileDownloadRequest\x1a(.dify.agent.stub.v1.FileDownloadResponseB\x1bZ\x19\x64ify/agent/stub/v1;stubv1b\x06proto3' ) _globals = globals() _builder.BuildMessageAndEnumDescriptors(DESCRIPTOR, _globals) _builder.BuildTopDescriptorsAndMessages(DESCRIPTOR, "dify.agent.stub.v1.agent_stub_pb2", _globals) if not _descriptor._USE_C_DESCRIPTORS: - DESCRIPTOR._loaded_options = None + _globals["DESCRIPTOR"]._loaded_options = None + _globals["DESCRIPTOR"]._serialized_options = b"Z\031dify/agent/stub/v1;stubv1" _globals["_CONNECTREQUEST"]._serialized_start = 59 _globals["_CONNECTREQUEST"]._serialized_end = 138 _globals["_CONNECTRESPONSE"]._serialized_start = 140 diff --git a/dify-agent/src/dify_agent/agent_stub/grpc/_generated/agent_stub_pb2.pyi b/dify-agent/src/dify_agent/agent_stub/grpc/_generated/agent_stub_pb2.pyi index 617956bd845..9f05ba759e5 100644 --- a/dify-agent/src/dify_agent/agent_stub/grpc/_generated/agent_stub_pb2.pyi +++ b/dify-agent/src/dify_agent/agent_stub/grpc/_generated/agent_stub_pb2.pyi @@ -49,12 +49,12 @@ class FileMapping(_message.Message): def __init__(self, transfer_method: _Optional[str] = ..., reference: _Optional[str] = ..., url: _Optional[str] = ...) -> None: ... class FileDownloadRequest(_message.Message): - __slots__ = ("file", "for_external") + __slots__ = ("file", "for_frontend") FILE_FIELD_NUMBER: _ClassVar[int] - FOR_EXTERNAL_FIELD_NUMBER: _ClassVar[int] + FOR_FRONTEND_FIELD_NUMBER: _ClassVar[int] file: FileMapping - for_external: bool - def __init__(self, file: _Optional[_Union[FileMapping, _Mapping]] = ..., for_external: _Optional[bool] = ...) -> None: ... + for_frontend: bool + def __init__(self, file: _Optional[_Union[FileMapping, _Mapping]] = ..., for_frontend: _Optional[bool] = ...) -> None: ... class FileDownloadResponse(_message.Message): __slots__ = ("filename", "mime_type", "size", "download_url") diff --git a/dify-agent/src/dify_agent/agent_stub/grpc/conversions.py b/dify-agent/src/dify_agent/agent_stub/grpc/conversions.py index a79e446c85e..72e5642c46e 100644 --- a/dify-agent/src/dify_agent/agent_stub/grpc/conversions.py +++ b/dify-agent/src/dify_agent/agent_stub/grpc/conversions.py @@ -101,7 +101,7 @@ def file_download_request_from_proto(message: agent_stub_pb2.FileDownloadRequest return AgentStubFileDownloadRequest.model_validate( { "file": file_mapping_kwargs, - "for_external": message.for_external if message.HasField("for_external") else True, + "for_frontend": message.for_frontend if message.HasField("for_frontend") else True, } ) @@ -110,7 +110,7 @@ def proto_file_download_request( pb2_module, *, file: AgentStubFileMapping, - for_external: bool = True, + for_frontend: bool = True, ) -> agent_stub_pb2.FileDownloadRequest: """Build one protobuf file-download request from the public DTO.""" mapping = pb2_module.FileMapping(transfer_method=file.transfer_method) @@ -119,7 +119,7 @@ def proto_file_download_request( if file.url is not None: mapping.url = file.url request = pb2_module.FileDownloadRequest(file=mapping) - request.for_external = for_external + request.for_frontend = for_frontend return request diff --git a/dify-agent/src/dify_agent/agent_stub/protocol/agent_stub.py b/dify-agent/src/dify_agent/agent_stub/protocol/agent_stub.py index 734ca50d53e..2287117bea3 100644 --- a/dify-agent/src/dify_agent/agent_stub/protocol/agent_stub.py +++ b/dify-agent/src/dify_agent/agent_stub/protocol/agent_stub.py @@ -15,7 +15,7 @@ from dataclasses import dataclass from typing import ClassVar, Final, Literal from urllib.parse import urlsplit, urlunsplit -from pydantic import BaseModel, ConfigDict, Field, JsonValue, model_validator +from pydantic import AliasChoices, BaseModel, ConfigDict, Field, JsonValue, model_validator from dify_agent.agent_stub._constants import AGENT_STUB_DRIVE_BASE_ENV_VAR, DEFAULT_AGENT_STUB_DRIVE_BASE @@ -246,10 +246,19 @@ class AgentStubFileMapping(BaseModel): class AgentStubFileDownloadRequest(BaseModel): - """Request body for one signed download URL allocation.""" + """Request one file URL for a specific consumer audience. + + ``for_frontend=True`` allocates a frontend-display URL that the CLI only + returns to its caller. ``False`` allocates a Sandbox byte-transfer URL that + the CLI immediately fetches. The deprecated HTTP input name + ``for_external`` remains accepted for one compatibility cycle. + """ file: AgentStubFileMapping - for_external: bool = True + for_frontend: bool = Field( + default=True, + validation_alias=AliasChoices("for_frontend", "for_external"), + ) model_config: ClassVar[ConfigDict] = ConfigDict(extra="forbid") diff --git a/dify-agent/src/dify_agent/agent_stub/server/agent_stub_files.py b/dify-agent/src/dify_agent/agent_stub/server/agent_stub_files.py index 59cd43b6bb5..541f5f62400 100644 --- a/dify-agent/src/dify_agent/agent_stub/server/agent_stub_files.py +++ b/dify-agent/src/dify_agent/agent_stub/server/agent_stub_files.py @@ -14,11 +14,12 @@ from __future__ import annotations from collections.abc import Mapping from dataclasses import dataclass +import posixpath from typing import Any, Protocol -from urllib.parse import urljoin +from urllib.parse import unquote, urlsplit import httpx -from pydantic import BaseModel, ConfigDict, ValidationError +from pydantic import ValidationError from dify_agent.agent_stub.protocol.agent_stub import ( AgentStubFileDownloadRequest, @@ -71,36 +72,28 @@ class AgentStubFileRequestError(RuntimeError): super().__init__(str(detail)) -class _BackwardsInvocationEnvelope(BaseModel): - """Minimal parser for Dify API plugin-style inner API envelopes.""" - - data: object | None = None - error: str | None = None - - model_config = ConfigDict(extra="ignore") - - @dataclass(slots=True) class DifyApiAgentStubFileRequestHandler: """Call Dify API inner file request endpoints on behalf of the sandbox. - The upload path calls ``/inner/api/upload/file/request`` and injects the + The upload path calls ``/inner/api/agent/files/upload-request`` and injects the authenticated execution context's ``tenant_id``, ``user_id``, and optional ``conversation_id`` along with the requested filename and mimetype. The download path calls - ``/inner/api/download/file/request`` and injects ``tenant_id``, + ``/inner/api/agent/files/download-request`` and injects ``tenant_id``, ``user_id``, ``user_from``, and ``invoke_from`` plus the validated public file mapping. ``user_id`` is mandatory for both operations. Missing user context is rejected before any network call with ``AgentStubFileRequestError(400, ...)``. - Timeouts, transport failures, non-2xx responses, invalid JSON, invalid - plugin-style envelopes, and invalid success schemas are all normalized into + Timeouts, transport failures, non-2xx responses, invalid JSON, and invalid + success schemas are all normalized into ``AgentStubFileRequestError`` so the stub routes can preserve a stable HTTP contract without exposing raw ``httpx`` or Pydantic exceptions. """ inner_api_url: str inner_api_key: str + sandbox_files_base_url: str timeout: httpx.Timeout | float = 30.0 async def create_upload_request( @@ -119,7 +112,7 @@ class DifyApiAgentStubFileRequestHandler: Raises: AgentStubFileRequestError: when user context is incomplete, the inner API times out or fails, the response is non-2xx, or the - success payload does not contain a non-empty ``url`` string. + success payload does not contain a valid ``upload_uri``. """ execution_context = self._require_user_context(principal.execution_context) payload = { @@ -129,12 +122,12 @@ class DifyApiAgentStubFileRequestHandler: "mimetype": request.mimetype, "conversation_id": execution_context.conversation_id, } - data = await self._post_inner_api("/inner/api/upload/file/request", payload) - upload_url = data.get("url") - if not isinstance(upload_url, str) or not upload_url: - raise AgentStubFileRequestError(502, "Dify API upload request response is missing url") + data = await self._post_inner_api("/inner/api/agent/files/upload-request", payload) + upload_uri = data.get("upload_uri") + if not isinstance(upload_uri, str) or not upload_uri: + raise AgentStubFileRequestError(502, "Dify API upload request response is missing upload_uri") return AgentStubFileUploadResponse( - upload_url=urljoin(f"{self.inner_api_url.rstrip('/')}/", upload_url), + upload_url=self._bind_sandbox_files_base_url(upload_uri), ) async def create_download_request( @@ -153,8 +146,7 @@ class DifyApiAgentStubFileRequestHandler: Raises: AgentStubFileRequestError: when user context is incomplete, the inner API times out or fails, the response is non-2xx, the - plugin-style envelope is malformed, or the success payload does - not match ``AgentStubFileDownloadResponse``. + success payload does not contain safe download metadata. """ execution_context = self._require_user_context(principal.execution_context) payload: dict[str, object] = { @@ -164,11 +156,14 @@ class DifyApiAgentStubFileRequestHandler: "invoke_from": execution_context.invoke_from, "file": request.file.model_dump(mode="json", exclude_none=True), } - if request.for_external is False: - payload["for_external"] = False - data = await self._post_inner_api("/inner/api/download/file/request", payload) + payload["for_frontend"] = request.for_frontend + data = await self._post_inner_api("/inner/api/agent/files/download-request", payload) + download_uri = data.get("download_uri") + if not isinstance(download_uri, str) or not download_uri: + raise AgentStubFileRequestError(502, "Dify API download request response is missing download_uri") + download_url = self._resolve_download_url(request=request, download_uri=download_uri) try: - return AgentStubFileDownloadResponse.model_validate(data) + return AgentStubFileDownloadResponse.model_validate({**data, "download_url": download_url}) except ValidationError as exc: raise AgentStubFileRequestError(502, "Dify API download request response is invalid") from exc @@ -197,15 +192,46 @@ class DifyApiAgentStubFileRequestHandler: if response.is_error: detail = raw_payload.get("detail", raw_payload) if isinstance(raw_payload, dict) else raw_payload raise AgentStubFileRequestError(response.status_code, detail) - try: - envelope = _BackwardsInvocationEnvelope.model_validate(raw_payload) - except ValidationError as exc: - raise AgentStubFileRequestError(502, "Dify API file request response is invalid") from exc - if envelope.error: - raise AgentStubFileRequestError(400, envelope.error) - if not isinstance(envelope.data, dict): - raise AgentStubFileRequestError(502, "Dify API file request response is missing data") - return dict(envelope.data) + if not isinstance(raw_payload, dict): + raise AgentStubFileRequestError(502, "Dify API file request response is invalid") + return raw_payload + + def _resolve_download_url(self, *, request: AgentStubFileDownloadRequest, download_uri: str) -> str: + if request.file.transfer_method == "remote_url": + if self._is_absolute_http_url(download_uri): + return download_uri + raise AgentStubFileRequestError(502, "Dify API returned an invalid remote download URL") + + if request.for_frontend: + if self._is_absolute_http_url(download_uri) or self._is_safe_dify_file_uri(download_uri): + return download_uri + raise AgentStubFileRequestError(502, "Dify API returned an unsafe frontend download URI") + + return self._bind_sandbox_files_base_url(download_uri) + + def _bind_sandbox_files_base_url(self, uri: str) -> str: + if not self._is_safe_dify_file_uri(uri): + raise AgentStubFileRequestError(502, "Dify API returned an unsafe Dify file URI") + return f"{self.sandbox_files_base_url.rstrip('/')}{uri}" + + @staticmethod + def _is_absolute_http_url(value: str) -> bool: + parsed = urlsplit(value) + return parsed.scheme in {"http", "https"} and bool(parsed.netloc) + + @staticmethod + def _is_safe_dify_file_uri(value: str) -> bool: + parsed = urlsplit(value) + if parsed.scheme or parsed.netloc or parsed.fragment or value.startswith("//"): + return False + + decoded_path = parsed.path + for _ in range(2): + decoded_path = unquote(decoded_path) + if "\\" in decoded_path: + return False + normalized_path = posixpath.normpath(decoded_path) + return decoded_path.startswith("/files/") and normalized_path.startswith("/files/") @staticmethod def _parse_json(response: httpx.Response) -> object: diff --git a/dify-agent/src/dify_agent/layers/_agent_file_cli_help.py b/dify-agent/src/dify_agent/layers/_agent_file_cli_help.py index 50deef3d851..cbe19a27bbe 100644 --- a/dify-agent/src/dify_agent/layers/_agent_file_cli_help.py +++ b/dify-agent/src/dify_agent/layers/_agent_file_cli_help.py @@ -3,5 +3,5 @@ AGENT_FILE_UPLOAD_REPLY_HINT = ( "When you want to provide a generated or sandbox-local file to the user in a " "natural-language reply, run the installed CLI command `dify-agent file upload PATH` and include the returned " - "`download_url` so the user can open or download the file." + "`public_download_url` so the user can open or download the file." ) diff --git a/dify-agent/src/dify_agent/server/settings.py b/dify-agent/src/dify_agent/server/settings.py index 37ad32f37ac..c8e388f275a 100644 --- a/dify-agent/src/dify_agent/server/settings.py +++ b/dify-agent/src/dify_agent/server/settings.py @@ -74,6 +74,10 @@ class ServerSettings(BaseSettings): e2b_shellctl_port: int = Field(default=5004, ge=1, le=65535) sandbox_file_upload_max_bytes: int = Field(default=50 * 1024 * 1024, ge=1) agent_stub_api_base_url: str | None = Field(default=None, validation_alias="DIFY_AGENT_STUB_API_BASE_URL") + sandbox_files_base_url: str | None = Field( + default=None, + validation_alias="DIFY_AGENT_SANDBOX_FILES_BASE_URL", + ) agent_stub_grpc_bind_address: str | None = Field(default=None, validation_alias="DIFY_AGENT_STUB_GRPC_BIND_ADDRESS") server_secret_key: str | None = None api_token: str | None = None @@ -107,6 +111,22 @@ class ServerSettings(BaseSettings): return normalize_agent_stub_api_base_url(validated) return normalize_agent_stub_api_base_url(stripped) + @field_validator("sandbox_files_base_url") + @classmethod + def normalize_sandbox_files_base_url_value(cls, value: str | None) -> str | None: + """Normalize the Dify API base URL reachable from the Sandbox.""" + + if value is None: + return None + stripped = value.strip() + if not stripped: + return None + validated = str(TypeAdapter(AnyHttpUrl).validate_python(stripped)) + parsed = validated.rstrip("/") + if "?" in parsed or "#" in parsed: + raise ValueError("DIFY_AGENT_SANDBOX_FILES_BASE_URL must not include a query string or fragment") + return parsed + @field_validator("agent_stub_grpc_bind_address") @classmethod def normalize_agent_stub_grpc_bind_address_value(cls, value: str | None) -> str | None: @@ -169,6 +189,14 @@ class ServerSettings(BaseSettings): """Require Agent Stub settings while allowing deployments without inner API calls.""" if self.agent_stub_api_base_url is not None and self.server_secret_key is None: raise ValueError("DIFY_AGENT_SERVER_SECRET_KEY is required when DIFY_AGENT_STUB_API_BASE_URL is set.") + if ( + self.agent_stub_api_base_url is not None + and self.inner_api_key is not None + and self.sandbox_files_base_url is None + ): + raise ValueError( + "DIFY_AGENT_SANDBOX_FILES_BASE_URL is required when Agent Stub file operations are enabled." + ) if self.agent_stub_grpc_bind_address is not None: if self.agent_stub_api_base_url is None: raise ValueError( @@ -209,12 +237,14 @@ class ServerSettings(BaseSettings): return AgentStubTokenCodec.from_server_secret(self.server_secret_key) def create_agent_stub_file_request_handler(self) -> DifyApiAgentStubFileRequestHandler | None: - """Return the Dify API file bridge when both Dify API settings are configured.""" - if self.inner_api_key is None: + """Return the file bridge when inner API and Sandbox data-plane settings are configured.""" + if self.inner_api_key is None or self.sandbox_files_base_url is None: return None return DifyApiAgentStubFileRequestHandler( inner_api_url=self.inner_api_url, inner_api_key=self.inner_api_key, + sandbox_files_base_url=self.sandbox_files_base_url, + timeout=self.create_outbound_http_timeout(), ) def create_agent_stub_config_request_handler(self) -> DifyApiAgentStubConfigRequestHandler | None: diff --git a/dify-agent/src/dify_agent/server/workspace_files.py b/dify-agent/src/dify_agent/server/workspace_files.py index 26e963f1092..6e6863fa035 100644 --- a/dify-agent/src/dify_agent/server/workspace_files.py +++ b/dify-agent/src/dify_agent/server/workspace_files.py @@ -114,7 +114,7 @@ class AgentStubWorkspaceFileUploader: mapping = AgentStubFileMapping(transfer_method="tool_file", reference=payload.reference) download_request = await self.file_request_handler.create_download_request( principal=principal, - request=AgentStubFileDownloadRequest(file=mapping, for_external=False), + request=AgentStubFileDownloadRequest(file=mapping, for_frontend=False), ) return WorkspaceUploadedFile( reference=payload.reference, diff --git a/dify-agent/tests/local/dify_agent/agent_stub/protocol/test_agent_stub_protocol.py b/dify-agent/tests/local/dify_agent/agent_stub/protocol/test_agent_stub_protocol.py index 0c16cf8bccf..45413319717 100644 --- a/dify-agent/tests/local/dify_agent/agent_stub/protocol/test_agent_stub_protocol.py +++ b/dify-agent/tests/local/dify_agent/agent_stub/protocol/test_agent_stub_protocol.py @@ -12,6 +12,7 @@ from dify_agent.agent_stub.protocol.agent_stub import ( AgentStubDriveCommitRequest, AgentStubDriveFileRef, AgentStubDriveManifestResponse, + AgentStubFileDownloadRequest, AgentStubFileMapping, agent_stub_connections_url, agent_stub_drive_base_for_ref, @@ -150,6 +151,21 @@ def test_agent_stub_file_mapping_rejects_remote_url_with_reference() -> None: ) +def test_agent_stub_file_download_request_accepts_legacy_http_audience_alias() -> None: + mapping = {"transfer_method": "tool_file", "reference": _reference("tool-file-1")} + + request = AgentStubFileDownloadRequest.model_validate({"file": mapping, "for_external": False}) + + assert request.for_frontend is False + assert request.model_dump() == { + "file": {"transfer_method": "tool_file", "reference": _reference("tool-file-1"), "url": None}, + "for_frontend": False, + } + + with pytest.raises(ValidationError): + _ = AgentStubFileDownloadRequest.model_validate({"file": mapping, "for_frontend": True, "for_external": False}) + + def test_agent_stub_drive_commit_request_validates_file_refs() -> None: request = AgentStubDriveCommitRequest( items=[ diff --git a/dify-agent/tests/local/dify_agent/agent_stub/protocol/test_grpc_conversions.py b/dify-agent/tests/local/dify_agent/agent_stub/protocol/test_grpc_conversions.py index 8c061254151..504ecccc857 100644 --- a/dify-agent/tests/local/dify_agent/agent_stub/protocol/test_grpc_conversions.py +++ b/dify-agent/tests/local/dify_agent/agent_stub/protocol/test_grpc_conversions.py @@ -53,32 +53,33 @@ def test_file_download_request_from_proto_respects_optional_reference() -> None: assert request.file.reference == _reference("tool-file-1") assert request.file.url is None - assert request.for_external is True + assert request.for_frontend is True -def test_file_download_request_from_proto_preserves_explicit_internal_audience() -> None: +def test_file_download_request_from_proto_preserves_explicit_sandbox_transfer_audience() -> None: message = agent_stub_pb2.FileDownloadRequest( file=agent_stub_pb2.FileMapping( transfer_method="tool_file", reference=_reference("tool-file-1"), ), - for_external=False, + for_frontend=False, ) request = file_download_request_from_proto(message) - assert request.for_external is False + assert request.for_frontend is False def test_proto_file_download_request_preserves_selected_audience() -> None: message = proto_file_download_request( agent_stub_pb2, file=AgentStubFileMapping(transfer_method="tool_file", reference=_reference("tool-file-1")), - for_external=False, + for_frontend=False, ) - assert message.HasField("for_external") is True - assert message.for_external is False + assert message.HasField("for_frontend") is True + assert message.for_frontend is False + assert agent_stub_pb2.FileDownloadRequest.DESCRIPTOR.fields_by_name["for_frontend"].number == 2 def test_connect_request_from_proto_rejects_invalid_metadata_json() -> None: diff --git a/dify-agent/tests/local/dify_agent/agent_stub/server/test_agent_stub_app.py b/dify-agent/tests/local/dify_agent/agent_stub/server/test_agent_stub_app.py index e683a7ea6db..8206a466e87 100644 --- a/dify-agent/tests/local/dify_agent/agent_stub/server/test_agent_stub_app.py +++ b/dify-agent/tests/local/dify_agent/agent_stub/server/test_agent_stub_app.py @@ -64,6 +64,7 @@ def test_create_agent_stub_app_wires_configured_file_handler_for_upload_requests server_secret_key=_base64url_secret(b"1" * 32), inner_api_url="https://api.example.com", inner_api_key="inner-secret", + sandbox_files_base_url="https://files.example.com", ) token_codec = settings.create_agent_stub_token_codec() assert token_codec is not None @@ -72,9 +73,9 @@ def test_create_agent_stub_app_wires_configured_file_handler_for_upload_requests original_async_client = httpx.AsyncClient def handler(request: httpx.Request) -> httpx.Response: - assert str(request.url) == "https://api.example.com/inner/api/upload/file/request" + assert str(request.url) == "https://api.example.com/inner/api/agent/files/upload-request" assert request.headers["X-Inner-Api-Key"] == "inner-secret" - return httpx.Response(200, json={"data": {"url": "https://files.example.com/upload"}}) + return httpx.Response(200, json={"upload_uri": "/files/upload/for-plugin?sign=1"}) monkeypatch.setattr( "dify_agent.agent_stub.server.agent_stub_files.httpx.AsyncClient", @@ -89,7 +90,7 @@ def test_create_agent_stub_app_wires_configured_file_handler_for_upload_requests ) assert response.status_code == 200 - assert response.json() == {"upload_url": "https://files.example.com/upload"} + assert response.json() == {"upload_url": "https://files.example.com/files/upload/for-plugin?sign=1"} def test_create_agent_stub_app_wires_configured_drive_handler_for_manifest_requests(monkeypatch) -> None: @@ -98,6 +99,7 @@ def test_create_agent_stub_app_wires_configured_drive_handler_for_manifest_reque server_secret_key=_base64url_secret(b"1" * 32), inner_api_url="https://api.example.com", inner_api_key="inner-secret", + sandbox_files_base_url="https://files.example.com", ) token_codec = settings.create_agent_stub_token_codec() assert token_codec is not None diff --git a/dify-agent/tests/local/dify_agent/agent_stub/server/test_agent_stub_files.py b/dify-agent/tests/local/dify_agent/agent_stub/server/test_agent_stub_files.py index 8782ba48af0..6fae0a6d0dd 100644 --- a/dify-agent/tests/local/dify_agent/agent_stub/server/test_agent_stub_files.py +++ b/dify-agent/tests/local/dify_agent/agent_stub/server/test_agent_stub_files.py @@ -5,6 +5,7 @@ import base64 import json import httpx +import pytest from dify_agent.agent_stub.protocol.agent_stub import ( AgentStubFileDownloadRequest, @@ -33,7 +34,7 @@ def _principal() -> AgentStubPrincipal: ) -def _patch_async_client(monkeypatch, handler) -> None: +def _patch_async_client(monkeypatch: pytest.MonkeyPatch, handler) -> None: original_async_client = httpx.AsyncClient monkeypatch.setattr( "dify_agent.agent_stub.server.agent_stub_files.httpx.AsyncClient", @@ -41,14 +42,22 @@ def _patch_async_client(monkeypatch, handler) -> None: ) +def _file_handler(*, sandbox_files_base_url: str = "https://sandbox-files.example.com/dify"): + return DifyApiAgentStubFileRequestHandler( + inner_api_url="https://api.internal.example.com", + inner_api_key="inner-secret", + sandbox_files_base_url=sandbox_files_base_url, + ) + + def _reference(record_id: str) -> str: payload = base64.urlsafe_b64encode(json.dumps({"record_id": record_id}, separators=(",", ":")).encode()).decode() return f"dify-file-ref:{payload}" -def test_dify_api_agent_stub_file_handler_injects_execution_context_for_upload(monkeypatch) -> None: +def test_upload_request_uses_agent_inner_endpoint_and_binds_sandbox_base(monkeypatch: pytest.MonkeyPatch) -> None: def handler(request: httpx.Request) -> httpx.Response: - assert str(request.url) == "https://api.example.com/inner/api/upload/file/request" + assert str(request.url) == "https://api.internal.example.com/inner/api/agent/files/upload-request" assert request.headers["X-Inner-Api-Key"] == "inner-secret" assert json.loads(request.content) == { "tenant_id": "tenant-1", @@ -57,240 +66,220 @@ def test_dify_api_agent_stub_file_handler_injects_execution_context_for_upload(m "mimetype": "application/pdf", "conversation_id": "conversation-1", } - return httpx.Response(200, json={"data": {"url": "https://files.example.com/upload"}}) + return httpx.Response(200, json={"upload_uri": "/files/upload/for-plugin?signed=yes"}) _patch_async_client(monkeypatch, handler) - file_handler = DifyApiAgentStubFileRequestHandler( - inner_api_url="https://api.example.com", - inner_api_key="inner-secret", - ) async def scenario() -> None: - response = await file_handler.create_upload_request( + response = await _file_handler().create_upload_request( principal=_principal(), request=AgentStubFileUploadRequest(filename="report.pdf", mimetype="application/pdf"), ) - assert response.upload_url == "https://files.example.com/upload" + assert response.upload_url == "https://sandbox-files.example.com/dify/files/upload/for-plugin?signed=yes" asyncio.run(scenario()) -def test_dify_api_agent_stub_file_handler_resolves_relative_upload_url(monkeypatch) -> None: - def handler(_request: httpx.Request) -> httpx.Response: - return httpx.Response(200, json={"data": {"url": "/files/upload/for-plugin?signed=yes"}}) +def test_sandbox_download_request_binds_origin_free_uri(monkeypatch: pytest.MonkeyPatch) -> None: + reference = _reference("tool-file-1") - _patch_async_client(monkeypatch, handler) - file_handler = DifyApiAgentStubFileRequestHandler( - inner_api_url="https://api.example.com", - inner_api_key="inner-secret", - ) - - async def scenario() -> None: - response = await file_handler.create_upload_request( - principal=_principal(), - request=AgentStubFileUploadRequest(filename="report.pdf", mimetype="application/pdf"), - ) - assert response.upload_url == "https://api.example.com/files/upload/for-plugin?signed=yes" - - asyncio.run(scenario()) - - -def test_dify_api_agent_stub_file_handler_injects_execution_context_for_download(monkeypatch) -> None: def handler(request: httpx.Request) -> httpx.Response: - assert str(request.url) == "https://api.example.com/inner/api/download/file/request" + assert str(request.url) == "https://api.internal.example.com/inner/api/agent/files/download-request" assert json.loads(request.content) == { "tenant_id": "tenant-1", "user_id": "user-1", "user_from": "account", "invoke_from": "service-api", - "file": {"transfer_method": "tool_file", "reference": _reference("tool-file-1")}, + "file": {"transfer_method": "tool_file", "reference": reference}, + "for_frontend": False, } return httpx.Response( 200, json={ - "data": { - "filename": "report.pdf", - "mime_type": "application/pdf", - "size": 123, - "download_url": "https://files.example.com/download", - } + "filename": "report.pdf", + "mime_type": "application/pdf", + "size": 123, + "download_uri": "/files/tools/tool-file-1.pdf?sign=1", }, ) _patch_async_client(monkeypatch, handler) - file_handler = DifyApiAgentStubFileRequestHandler( - inner_api_url="https://api.example.com", - inner_api_key="inner-secret", - ) async def scenario() -> None: - response = await file_handler.create_download_request( + response = await _file_handler().create_download_request( principal=_principal(), request=AgentStubFileDownloadRequest( - file=AgentStubFileMapping(transfer_method="tool_file", reference=_reference("tool-file-1")) + file=AgentStubFileMapping(transfer_method="tool_file", reference=reference), + for_frontend=False, ), ) - assert response.download_url == "https://files.example.com/download" + assert response.download_url == "https://sandbox-files.example.com/dify/files/tools/tool-file-1.pdf?sign=1" asyncio.run(scenario()) -def test_dify_api_agent_stub_file_handler_forwards_internal_download_audience(monkeypatch) -> None: +@pytest.mark.parametrize( + "download_uri", + [ + "https://dify.example.com/files/tools/tool-file-1.pdf?sign=1", + "/files/tools/tool-file-1.pdf?sign=1", + ], +) +def test_frontend_download_request_preserves_public_or_relative_uri( + monkeypatch: pytest.MonkeyPatch, + download_uri: str, +) -> None: def handler(request: httpx.Request) -> httpx.Response: - assert str(request.url) == "https://api.example.com/inner/api/download/file/request" - assert json.loads(request.content)["for_external"] is False + assert json.loads(request.content)["for_frontend"] is True return httpx.Response( 200, - json={ - "data": { - "filename": "report.pdf", - "mime_type": "application/pdf", - "size": 123, - "download_url": "http://internal-files/report.pdf", - } - }, + json={"filename": "report.pdf", "mime_type": "application/pdf", "size": 123, "download_uri": download_uri}, ) _patch_async_client(monkeypatch, handler) - file_handler = DifyApiAgentStubFileRequestHandler( - inner_api_url="https://api.example.com", - inner_api_key="inner-secret", - ) async def scenario() -> None: - response = await file_handler.create_download_request( + response = await _file_handler().create_download_request( principal=_principal(), request=AgentStubFileDownloadRequest( file=AgentStubFileMapping(transfer_method="tool_file", reference=_reference("tool-file-1")), - for_external=False, + for_frontend=True, ), ) - assert response.download_url == "http://internal-files/report.pdf" + assert response.download_url == download_uri asyncio.run(scenario()) -def test_dify_api_agent_stub_file_handler_rejects_missing_user_id() -> None: - file_handler = DifyApiAgentStubFileRequestHandler( - inner_api_url="https://api.example.com", - inner_api_key="inner-secret", - ) +def test_remote_download_url_is_never_rewritten(monkeypatch: pytest.MonkeyPatch) -> None: + remote_url = "https://remote.example.com/report.pdf" + + def handler(_request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={"filename": "report.pdf", "mime_type": "application/pdf", "size": 123, "download_uri": remote_url}, + ) + + _patch_async_client(monkeypatch, handler) + + async def scenario() -> None: + response = await _file_handler().create_download_request( + principal=_principal(), + request=AgentStubFileDownloadRequest( + file=AgentStubFileMapping(transfer_method="remote_url", url=remote_url), + for_frontend=False, + ), + ) + assert response.download_url == remote_url + + asyncio.run(scenario()) + + +def test_remote_download_rejects_relative_uri_for_frontend_audience(monkeypatch: pytest.MonkeyPatch) -> None: + def handler(_request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={"filename": "report.pdf", "mime_type": "application/pdf", "size": 123, "download_uri": "/files/x"}, + ) + + _patch_async_client(monkeypatch, handler) + + async def scenario() -> None: + with pytest.raises(AgentStubFileRequestError, match="invalid remote download URL"): + await _file_handler().create_download_request( + principal=_principal(), + request=AgentStubFileDownloadRequest( + file=AgentStubFileMapping( + transfer_method="remote_url", url="https://remote.example.com/report.pdf" + ), + for_frontend=True, + ), + ) + + asyncio.run(scenario()) + + +@pytest.mark.parametrize( + "unsafe_uri", + [ + "//attacker.example/files/x", + "/files/../admin", + "/files/%252e%252e/admin", + "http://api:5001/files/tools/x", + "/not-files/x", + ], +) +def test_sandbox_download_rejects_unsafe_dify_file_uri( + monkeypatch: pytest.MonkeyPatch, + unsafe_uri: str, +) -> None: + def handler(_request: httpx.Request) -> httpx.Response: + return httpx.Response( + 200, + json={"filename": "x", "mime_type": None, "size": 1, "download_uri": unsafe_uri}, + ) + + _patch_async_client(monkeypatch, handler) + + async def scenario() -> None: + with pytest.raises(AgentStubFileRequestError, match="unsafe Dify file URI"): + await _file_handler().create_download_request( + principal=_principal(), + request=AgentStubFileDownloadRequest( + file=AgentStubFileMapping(transfer_method="tool_file", reference=_reference("tool-file-1")), + for_frontend=False, + ), + ) + + asyncio.run(scenario()) + + +def test_handler_rejects_missing_execution_user_before_network() -> None: principal = _principal() principal.execution_context = principal.execution_context.model_copy(update={"user_id": None}) async def scenario() -> None: - try: - await file_handler.create_upload_request( + with pytest.raises(AgentStubFileRequestError, match="user_id"): + await _file_handler().create_upload_request( principal=principal, request=AgentStubFileUploadRequest(filename="report.pdf", mimetype="application/pdf"), ) - except AgentStubFileRequestError as exc: - assert "user_id" in str(exc) - else: - raise AssertionError("expected AgentStubFileRequestError") asyncio.run(scenario()) -def test_dify_api_agent_stub_file_handler_maps_non_2xx_response(monkeypatch) -> None: +def test_handler_preserves_inner_api_error_status(monkeypatch: pytest.MonkeyPatch) -> None: def handler(_request: httpx.Request) -> httpx.Response: return httpx.Response(403, json={"detail": "forbidden"}) _patch_async_client(monkeypatch, handler) - file_handler = DifyApiAgentStubFileRequestHandler( - inner_api_url="https://api.example.com", - inner_api_key="inner-secret", - ) async def scenario() -> None: - try: - await file_handler.create_upload_request( + with pytest.raises(AgentStubFileRequestError) as exc_info: + await _file_handler().create_upload_request( principal=_principal(), request=AgentStubFileUploadRequest(filename="report.pdf", mimetype="application/pdf"), ) - except AgentStubFileRequestError as exc: - assert exc.status_code == 403 - assert exc.detail == "forbidden" - else: - raise AssertionError("expected AgentStubFileRequestError") + assert exc_info.value.status_code == 403 + assert exc_info.value.detail == "forbidden" asyncio.run(scenario()) -def test_dify_api_agent_stub_file_handler_maps_error_envelope(monkeypatch) -> None: +def test_handler_rejects_missing_download_uri(monkeypatch: pytest.MonkeyPatch) -> None: def handler(_request: httpx.Request) -> httpx.Response: - return httpx.Response(200, json={"error": "bad request"}) + return httpx.Response(200, json={"filename": "report.pdf", "size": 1}) _patch_async_client(monkeypatch, handler) - file_handler = DifyApiAgentStubFileRequestHandler( - inner_api_url="https://api.example.com", - inner_api_key="inner-secret", - ) async def scenario() -> None: - try: - await file_handler.create_download_request( + with pytest.raises(AgentStubFileRequestError, match="missing download_uri"): + await _file_handler().create_download_request( principal=_principal(), request=AgentStubFileDownloadRequest( - file=AgentStubFileMapping(transfer_method="tool_file", reference=_reference("tool-file-1")) + file=AgentStubFileMapping(transfer_method="tool_file", reference=_reference("tool-file-1")), + for_frontend=False, ), ) - except AgentStubFileRequestError as exc: - assert exc.status_code == 400 - assert exc.detail == "bad request" - else: - raise AssertionError("expected AgentStubFileRequestError") - - asyncio.run(scenario()) - - -def test_dify_api_agent_stub_file_handler_rejects_upload_response_missing_url(monkeypatch) -> None: - def handler(_request: httpx.Request) -> httpx.Response: - return httpx.Response(200, json={"data": {}}) - - _patch_async_client(monkeypatch, handler) - file_handler = DifyApiAgentStubFileRequestHandler( - inner_api_url="https://api.example.com", - inner_api_key="inner-secret", - ) - - async def scenario() -> None: - try: - await file_handler.create_upload_request( - principal=_principal(), - request=AgentStubFileUploadRequest(filename="report.pdf", mimetype="application/pdf"), - ) - except AgentStubFileRequestError as exc: - assert exc.status_code == 502 - assert exc.detail == "Dify API upload request response is missing url" - else: - raise AssertionError("expected AgentStubFileRequestError") - - asyncio.run(scenario()) - - -def test_dify_api_agent_stub_file_handler_rejects_invalid_download_response_schema(monkeypatch) -> None: - def handler(_request: httpx.Request) -> httpx.Response: - return httpx.Response(200, json={"data": {"filename": "report.pdf"}}) - - _patch_async_client(monkeypatch, handler) - file_handler = DifyApiAgentStubFileRequestHandler( - inner_api_url="https://api.example.com", - inner_api_key="inner-secret", - ) - - async def scenario() -> None: - try: - await file_handler.create_download_request( - principal=_principal(), - request=AgentStubFileDownloadRequest( - file=AgentStubFileMapping(transfer_method="tool_file", reference=_reference("tool-file-1")) - ), - ) - except AgentStubFileRequestError as exc: - assert exc.status_code == 502 - assert exc.detail == "Dify API download request response is invalid" - else: - raise AssertionError("expected AgentStubFileRequestError") asyncio.run(scenario()) diff --git a/dify-agent/tests/local/dify_agent/agent_stub/server/test_grpc_service.py b/dify-agent/tests/local/dify_agent/agent_stub/server/test_grpc_service.py index 149ce1cbffe..b179dc70921 100644 --- a/dify-agent/tests/local/dify_agent/agent_stub/server/test_grpc_service.py +++ b/dify-agent/tests/local/dify_agent/agent_stub/server/test_grpc_service.py @@ -130,7 +130,7 @@ def test_agent_stub_grpc_transport_delegates_file_download_requests() -> None: async def create_download_request(self, *, principal, request): assert principal.execution_context.user_id == "user-1" assert request.file.reference == _reference("tool-file-1") - assert request.for_external is False + assert request.for_frontend is False return type( "Response", (), @@ -160,7 +160,7 @@ def test_agent_stub_grpc_transport_delegates_file_download_requests() -> None: transfer_method="tool_file", reference=_reference("tool-file-1"), ), - for_external=False, + for_frontend=False, ), metadata=(("authorization", f"Bearer {token}"),), ) diff --git a/dify-agent/tests/local/dify_agent/server/test_app.py b/dify-agent/tests/local/dify_agent/server/test_app.py index 1891623b38d..86c2f95dc63 100644 --- a/dify-agent/tests/local/dify_agent/server/test_app.py +++ b/dify-agent/tests/local/dify_agent/server/test_app.py @@ -192,6 +192,7 @@ def test_create_app_creates_scheduler_and_closes_after_shutdown(monkeypatch: pyt plugin_daemon_api_key="daemon-secret", inner_api_url="http://dify-api", inner_api_key="inner-secret", + sandbox_files_base_url="http://api:5001", local_sandbox_endpoint="http://shellctl", local_sandbox_auth_token="shell-secret", agent_stub_api_base_url="https://agent.example.com/agent-stub", @@ -230,7 +231,9 @@ def test_create_app_creates_scheduler_and_closes_after_shutdown(monkeypatch: pyt assert execution_context_layer.daemon_api_key == "daemon-secret" assert shell_layer.agent_stub_token_factory is not None token = shell_layer.agent_stub_token_factory(_execution_context(), session_id="abc12ff") - decoded = settings.create_agent_stub_token_codec().decode_token(token) + token_codec = settings.create_agent_stub_token_codec() + assert token_codec is not None + decoded = token_codec.decode_token(token) assert decoded.execution_context == _execution_context() assert decoded.session_id == "abc12ff" knowledge_provider = next(provider for provider in layer_providers if provider.type_id == "dify.knowledge_base") @@ -318,6 +321,7 @@ def test_create_app_wires_authenticated_agent_stub_file_upload_route(monkeypatch server_secret_key=_base64url_secret(b"1" * 32), inner_api_url="https://api.example.com", inner_api_key="inner-secret", + sandbox_files_base_url="https://files.example.com", ) token_codec = settings.create_agent_stub_token_codec() assert token_codec is not None @@ -326,9 +330,9 @@ def test_create_app_wires_authenticated_agent_stub_file_upload_route(monkeypatch original_async_client = httpx.AsyncClient def handler(request: httpx.Request) -> httpx.Response: - assert str(request.url) == "https://api.example.com/inner/api/upload/file/request" + assert str(request.url) == "https://api.example.com/inner/api/agent/files/upload-request" assert request.headers["X-Inner-Api-Key"] == "inner-secret" - return httpx.Response(200, json={"data": {"url": "https://files.example.com/upload"}}) + return httpx.Response(200, json={"upload_uri": "/files/upload/for-plugin?sign=1"}) monkeypatch.setattr( "dify_agent.agent_stub.server.agent_stub_files.httpx.AsyncClient", @@ -343,7 +347,7 @@ def test_create_app_wires_authenticated_agent_stub_file_upload_route(monkeypatch ) assert response.status_code == 200 - assert response.json() == {"upload_url": "https://files.example.com/upload"} + assert response.json() == {"upload_url": "https://files.example.com/files/upload/for-plugin?sign=1"} assert FakeRunScheduler.created[0].shutdown_called is True assert fake_http_client.is_closed is True assert fake_redis.closed is True @@ -357,6 +361,7 @@ def test_create_app_wires_authenticated_agent_stub_drive_manifest_route(monkeypa server_secret_key=_base64url_secret(b"1" * 32), inner_api_url="https://api.example.com", inner_api_key="inner-secret", + sandbox_files_base_url="https://files.example.com", ) token_codec = settings.create_agent_stub_token_codec() assert token_codec is not None diff --git a/dify-agent/tests/local/dify_agent/server/test_settings.py b/dify-agent/tests/local/dify_agent/server/test_settings.py index d4c9af76a8b..f669dc0f226 100644 --- a/dify-agent/tests/local/dify_agent/server/test_settings.py +++ b/dify-agent/tests/local/dify_agent/server/test_settings.py @@ -71,11 +71,13 @@ def test_server_settings_defaults_shellctl_auth_token_to_none( def test_server_settings_reads_agent_stub_settings_from_env(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("DIFY_AGENT_STUB_API_BASE_URL", "https://agent.example.com/agent-stub/") + monkeypatch.setenv("DIFY_AGENT_SANDBOX_FILES_BASE_URL", "https://dify.example.com/prefix/") monkeypatch.setenv("DIFY_AGENT_SERVER_SECRET_KEY", _base64url_secret(secrets.token_bytes(32))) settings = ServerSettings() assert settings.agent_stub_api_base_url == "https://agent.example.com/agent-stub" + assert settings.sandbox_files_base_url == "https://dify.example.com/prefix" def test_server_settings_normalizes_agent_stub_service_root_from_env(monkeypatch: pytest.MonkeyPatch) -> None: @@ -125,6 +127,23 @@ def test_server_settings_rejects_public_agent_stub_api_base_url_without_secret_k _ = ServerSettings(agent_stub_api_base_url="https://agent.example.com/agent-stub") +def test_server_settings_requires_sandbox_files_base_url_for_agent_stub_file_operations() -> None: + with pytest.raises(ValidationError, match="DIFY_AGENT_SANDBOX_FILES_BASE_URL"): + _ = ServerSettings( + agent_stub_api_base_url="https://agent.example.com/agent-stub", + inner_api_key="inner-secret", + server_secret_key=_base64url_secret(secrets.token_bytes(32)), + ) + + +def test_server_settings_rejects_sandbox_files_base_url_query_or_fragment() -> None: + with pytest.raises(ValidationError, match="query string or fragment"): + _ = ServerSettings(sandbox_files_base_url="https://dify.example.com?x=1") + + with pytest.raises(ValidationError, match="query string or fragment"): + _ = ServerSettings(sandbox_files_base_url="https://dify.example.com#fragment") + + def test_server_settings_accepts_grpc_agent_stub_api_base_url_and_bind_override() -> None: settings = ServerSettings( agent_stub_api_base_url="grpc://agent.example.com:9091", @@ -210,6 +229,7 @@ def test_server_settings_create_agent_stub_file_request_handler_returns_handler_ settings = ServerSettings( inner_api_url="https://api.example.com", inner_api_key="inner-secret", + sandbox_files_base_url="https://sandbox-files.example.com/dify", ) handler = settings.create_agent_stub_file_request_handler() @@ -217,6 +237,7 @@ def test_server_settings_create_agent_stub_file_request_handler_returns_handler_ assert isinstance(handler, DifyApiAgentStubFileRequestHandler) assert handler.inner_api_url == "https://api.example.com" assert handler.inner_api_key == "inner-secret" + assert handler.sandbox_files_base_url == "https://sandbox-files.example.com/dify" def test_server_settings_create_agent_stub_drive_request_handler_returns_none_without_full_settings() -> None: diff --git a/docker/.env.example b/docker/.env.example index abf69363047..d7ac8555e55 100644 --- a/docker/.env.example +++ b/docker/.env.example @@ -283,6 +283,9 @@ DIFY_AGENT_E2B_TEMPLATE=difys-default-team/dify-agent-local-sandbox DIFY_AGENT_E2B_ACTIVE_TIMEOUT_SECONDS=3600 DIFY_AGENT_E2B_SHELLCTL_AUTH_TOKEN= DIFY_AGENT_E2B_SHELLCTL_PORT=5004 +# Sandbox-reachable Dify API base for dify-agent CLI /files/* transfers. +# Remote Sandboxes should use the public Dify ingress; local Compose uses api via agent_ssrf_proxy. +DIFY_AGENT_SANDBOX_FILES_BASE_URL=http://api:5001 DIFY_AGENT_STUB_API_BASE_URL=http://agent_backend:5050/agent-stub # This is security-sensitive: it derives the JWE encryption key for Agent Stub bearer tokens. # Replace this development default in production. diff --git a/docker/docker-compose-template.yaml b/docker/docker-compose-template.yaml index 856d9d99787..0c864209cad 100644 --- a/docker/docker-compose-template.yaml +++ b/docker/docker-compose-template.yaml @@ -681,6 +681,7 @@ services: DIFY_AGENT_E2B_SHELLCTL_PORT: ${DIFY_AGENT_E2B_SHELLCTL_PORT:-5004} DIFY_AGENT_SANDBOX_FILE_UPLOAD_MAX_BYTES: ${PLUGIN_MAX_FILE_SIZE:-52428800} DIFY_AGENT_STUB_API_BASE_URL: ${DIFY_AGENT_STUB_API_BASE_URL:-http://agent_backend:5050/agent-stub} + DIFY_AGENT_SANDBOX_FILES_BASE_URL: ${DIFY_AGENT_SANDBOX_FILES_BASE_URL:-http://api:5001} # This is security-sensitive: it derives the JWE encryption key for Agent Stub bearer tokens. # Replace this development default in production. # Generate one with: python -c 'import secrets; print(secrets.token_urlsafe(32))' diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml index 475225f3b1f..ef3f92a3ae9 100644 --- a/docker/docker-compose.yaml +++ b/docker/docker-compose.yaml @@ -687,6 +687,7 @@ services: DIFY_AGENT_E2B_SHELLCTL_PORT: ${DIFY_AGENT_E2B_SHELLCTL_PORT:-5004} DIFY_AGENT_SANDBOX_FILE_UPLOAD_MAX_BYTES: ${PLUGIN_MAX_FILE_SIZE:-52428800} DIFY_AGENT_STUB_API_BASE_URL: ${DIFY_AGENT_STUB_API_BASE_URL:-http://agent_backend:5050/agent-stub} + DIFY_AGENT_SANDBOX_FILES_BASE_URL: ${DIFY_AGENT_SANDBOX_FILES_BASE_URL:-http://api:5001} # This is security-sensitive: it derives the JWE encryption key for Agent Stub bearer tokens. # Replace this development default in production. # Generate one with: python -c 'import secrets; print(secrets.token_urlsafe(32))' diff --git a/docker/envs/core-services/dify-agent.env.example b/docker/envs/core-services/dify-agent.env.example index d434ecf5945..aa96908531b 100644 --- a/docker/envs/core-services/dify-agent.env.example +++ b/docker/envs/core-services/dify-agent.env.example @@ -35,6 +35,8 @@ DIFY_AGENT_E2B_SHELLCTL_PORT=5004 # Standalone/direct-container byte limit. Full Docker Compose derives this from # PLUGIN_MAX_FILE_SIZE in docker/.env. DIFY_AGENT_SANDBOX_FILE_UPLOAD_MAX_BYTES=52428800 +# Sandbox-reachable Dify API base for signed /files/* transfers. +DIFY_AGENT_SANDBOX_FILES_BASE_URL=http://api:5001 DIFY_AGENT_STUB_API_BASE_URL=http://agent_backend:5050/agent-stub # This is security-sensitive: it derives the JWE encryption key for Agent Stub bearer tokens. # Replace this development default in production. diff --git a/docker/ssrf_proxy/test_ssrf_proxy_config.sh b/docker/ssrf_proxy/test_ssrf_proxy_config.sh index 0c3aecda0ae..e2e6f435b49 100755 --- a/docker/ssrf_proxy/test_ssrf_proxy_config.sh +++ b/docker/ssrf_proxy/test_ssrf_proxy_config.sh @@ -39,6 +39,24 @@ http_code_for() { printf '%s\n' "$output" | awk '$1 ~ /^HTTP\// { code = $2 } END { print code }' } +http_code_for_post() { + local proxy_url="$1" + local target_url="$2" + local output + + output="$( + docker run \ + --rm \ + --network "$NETWORK_NAME" \ + --env "http_proxy=$proxy_url" \ + --env "https_proxy=$proxy_url" \ + "$CLIENT_IMAGE" \ + wget -S -O /dev/null -T 10 --post-data=file-bytes "$target_url" 2>&1 || true + )" + + printf '%s\n' "$output" | awk '$1 ~ /^HTTP\// { code = $2 } END { print code }' +} + direct_http_code_for() { local target_url="$1" local output @@ -80,6 +98,19 @@ assert_public_target_allowed() { fi } +assert_post_target_not_blocked() { + local proxy_url="$1" + local target_url="$2" + local status_code + + status_code="$(http_code_for_post "$proxy_url" "$target_url")" + if [[ -z "$status_code" || "$status_code" == "403" ]]; then + echo "Expected POST $target_url to pass the proxy ACL, got ${status_code:-no response}." + docker logs "$AGENT_PROXY_CONTAINER_NAME" >&2 || true + exit 1 + fi +} + assert_sandbox_bridge_allowed() { local target_url="$1" local status_code @@ -214,6 +245,8 @@ assert_private_target_blocked "$agent_proxy_url" "http://agent_backend:5050/inde # api /files/* must be allowed. assert_public_target_allowed "$agent_proxy_url" "http://api:5001/files/test" +assert_public_target_allowed "$agent_proxy_url" "http://api:5001/files/test?timestamp=1&nonce=2&sign=3" +assert_post_target_not_blocked "$agent_proxy_url" "http://api:5001/files/upload/for-plugin?timestamp=1&nonce=2&sign=3" # api non-/files paths must be blocked. assert_private_target_blocked "$agent_proxy_url" "http://api:5001/index.html"