From c64d9061ce336070167a62fecec2cff1be616533 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9E=97=E7=8E=AE=20=28Jade=20Lin=29?= Date: Wed, 14 Oct 2026 02:34:16 +0000 Subject: [PATCH] feat(api): publish marketplace templates as ifpkg (#43990) --- api/core/helper/creators.py | 28 ++-- api/services/agent/roster_package_entities.py | 2 +- api/services/app/console_service.py | 13 +- api/services/app/creators_platform_gateway.py | 6 +- .../unit_tests/core/helper/test_creators.py | 157 +++++++++--------- .../services/app/test_console_service.py | 51 +++++- .../app/test_creators_platform_gateway.py | 11 ++ 7 files changed, 160 insertions(+), 108 deletions(-) diff --git a/api/core/helper/creators.py b/api/core/helper/creators.py index 73f7fa536b4..0e6ed2b0c18 100644 --- a/api/core/helper/creators.py +++ b/api/core/helper/creators.py @@ -1,31 +1,33 @@ """ Helper module for Creators Platform integration. -Provides functionality to upload DSL files to the Creators Platform -and generate redirect URLs with OAuth authorization codes. +Uploads application packages and generates redirect URLs with claim codes +and optional OAuth authorization codes. """ -import logging +from typing import BinaryIO from urllib.parse import urlencode import httpx from yarl import URL from configs import dify_config +from core.helper import ssrf_proxy -logger = logging.getLogger(__name__) - -creators_platform_api_url = URL(str(dify_config.CREATORS_PLATFORM_API_URL)) - -_CREATORS_REQUEST_TIMEOUT: httpx.Timeout = httpx.Timeout(30.0, connect=5.0) +_CREATORS_REQUEST_TIMEOUT = httpx.Timeout(30.0, connect=5.0) -def upload_dsl(dsl_file_bytes: bytes, filename: str = "template.yaml") -> str: - url = str(creators_platform_api_url / "api/v1/templates/anonymous-upload") - response = httpx.post(url, files={"file": (filename, dsl_file_bytes)}, timeout=_CREATORS_REQUEST_TIMEOUT) +def upload_package(source: BinaryIO, *, filename: str) -> str: + url = str(URL(str(dify_config.CREATORS_PLATFORM_API_URL)) / "api/v1/templates/anonymous-upload") + response = ssrf_proxy.post( + url, + files={"file": (filename, source, "application/octet-stream")}, + timeout=_CREATORS_REQUEST_TIMEOUT, + max_retries=0, + ) response.raise_for_status() - data = response.json() - claim_code = data.get("data", {}).get("claim_code") + data = response.json().get("data") + claim_code = data.get("claim_code") if isinstance(data, dict) else None if not isinstance(claim_code, str) or not claim_code: raise ValueError("Creators Platform did not return a valid claim_code") return claim_code diff --git a/api/services/agent/roster_package_entities.py b/api/services/agent/roster_package_entities.py index 8534cc9e5ec..c56135c3b2a 100644 --- a/api/services/agent/roster_package_entities.py +++ b/api/services/agent/roster_package_entities.py @@ -202,7 +202,7 @@ class PreparedRosterAgentPackage(PreparedPackageArchive): @dataclass class RosterAgentPackageExport: - """Download artifact owning its archive until the response closes.""" + """Export artifact owning its archive until its consumer closes it.""" archive: BinaryIO filename: str diff --git a/api/services/app/console_service.py b/api/services/app/console_service.py index 4e2f39e78d1..795132285f9 100644 --- a/api/services/app/console_service.py +++ b/api/services/app/console_service.py @@ -157,7 +157,7 @@ class AppTransfers(Protocol): class CreatorsPlatform(Protocol): def require_enabled(self) -> None: ... - def upload(self, dsl: str) -> str: ... + def upload(self, source: BinaryIO, *, filename: str) -> str: ... def authorize(self, account_id: str) -> str | None: ... @@ -395,6 +395,11 @@ class ConsoleAppService: raise AppExportPaidPlanRequiredError("This feature requires a paid plan.") if options.format == "yaml": return self._transfers.export_dsl(context, app.id, options) + return self._export_package(context, app, options) + + def _export_package( + self, context: RequestContext, app: AppReference, options: AppExportOptions + ) -> RosterAgentPackageExport: if app.mode == "agent": if app.bound_agent_id is None: raise AppExportAgentNotFoundError("Agent not found") @@ -407,10 +412,10 @@ class ConsoleAppService: def publish(self, context: RequestContext, app_id: str) -> str: # Validate ownership before producing external effects, including feature rejection. - self._apps.get_reference(context, app_id) + app = self._apps.get_reference(context, app_id) self._creators.require_enabled() - dsl = self._transfers.export_dsl(context, app_id, AppExportOptions(include_secret=False)) - claim_code = self._creators.upload(dsl) + with self._export_package(context, app, AppExportOptions(format="ifpkg", include_secret=False)) as package: + claim_code = self._creators.upload(package.archive, filename=package.filename) code = self._creators.authorize(context.account_id) return self._creators.redirect_url(claim_code, code) diff --git a/api/services/app/creators_platform_gateway.py b/api/services/app/creators_platform_gateway.py index 5c52ba8b1a2..162fdb171f2 100644 --- a/api/services/app/creators_platform_gateway.py +++ b/api/services/app/creators_platform_gateway.py @@ -1,6 +1,6 @@ """Creators Platform uploads, redirects and OAuth authorization adapter.""" -from typing import override +from typing import BinaryIO, override from configs import dify_config from core.helper import creators @@ -18,8 +18,8 @@ class CreatorsPlatformGateway(CreatorsPlatform): raise CreatorsPlatformDisabledError("Creators Platform features are not enabled") @override - def upload(self, dsl: str) -> str: - return creators.upload_dsl(dsl.encode("utf-8")) + def upload(self, source: BinaryIO, *, filename: str) -> str: + return creators.upload_package(source, filename=filename) @override def authorize(self, account_id: str) -> str | None: diff --git a/api/tests/unit_tests/core/helper/test_creators.py b/api/tests/unit_tests/core/helper/test_creators.py index 64ff7825431..a7bf4d95e9d 100644 --- a/api/tests/unit_tests/core/helper/test_creators.py +++ b/api/tests/unit_tests/core/helper/test_creators.py @@ -1,92 +1,18 @@ """Tests for the Creators Platform helper module.""" +from collections.abc import Callable +from io import BytesIO from unittest.mock import patch +from zipfile import ZIP_DEFLATED, ZipFile import httpx import pytest -from yarl import URL - -@pytest.fixture(autouse=True) -def _patch_creators_url(monkeypatch: pytest.MonkeyPatch): - """Patch the module-level creators_platform_api_url for all tests.""" - monkeypatch.setattr( - "core.helper.creators.creators_platform_api_url", - URL("https://creators.example.com"), - ) - - -class TestUploadDSL: - @patch("core.helper.creators.httpx.post") - def test_returns_claim_code(self, mock_post): - mock_post.return_value = httpx.Response( - 200, - request=httpx.Request("POST", "https://creators.example.com/api/v1/templates/anonymous-upload"), - json={"data": {"claim_code": "abc123"}}, - ) - - from core.helper.creators import upload_dsl - - result = upload_dsl(b"app: demo", "demo.yaml") - - assert result == "abc123" - mock_post.assert_called_once() - call_kwargs = mock_post.call_args - assert "anonymous-upload" in call_kwargs.args[0] - timeout = call_kwargs.kwargs["timeout"] - assert isinstance(timeout, httpx.Timeout) - assert timeout.connect == 5.0 - assert timeout.read == 30.0 - - def test_creators_request_timeout_bounds_connect_phase(self): - """Module-level _CREATORS_REQUEST_TIMEOUT should bound the connect phase to 5.0s.""" - from core.helper.creators import _CREATORS_REQUEST_TIMEOUT - - assert _CREATORS_REQUEST_TIMEOUT.connect == 5.0 - assert _CREATORS_REQUEST_TIMEOUT.read == 30.0 - - @patch("core.helper.creators.httpx.post") - def test_raises_on_missing_claim_code(self, mock_post): - mock_post.return_value = httpx.Response( - 200, - request=httpx.Request("POST", "https://creators.example.com/api/v1/templates/anonymous-upload"), - json={"data": {}}, - ) - - from core.helper.creators import upload_dsl - - with pytest.raises(ValueError, match="claim_code"): - upload_dsl(b"app: demo") - - @patch("core.helper.creators.httpx.post") - def test_raises_on_non_string_claim_code(self, mock_post): - mock_post.return_value = httpx.Response( - 200, - request=httpx.Request("POST", "https://creators.example.com/api/v1/templates/anonymous-upload"), - json={"data": {"claim_code": 123}}, - ) - - from core.helper.creators import upload_dsl - - with pytest.raises(ValueError, match="claim_code"): - upload_dsl(b"app: demo") - - @patch("core.helper.creators.httpx.post") - def test_raises_on_http_error(self, mock_post): - mock_post.return_value = httpx.Response( - 500, - request=httpx.Request("POST", "https://creators.example.com/api/v1/templates/anonymous-upload"), - ) - - from core.helper.creators import upload_dsl - - with pytest.raises(httpx.HTTPStatusError) as caught: - upload_dsl(b"app: demo") - assert caught.value.response is mock_post.return_value +from core.helper import creators class TestGetRedirectUrl: - def test_without_oauth_code(self, config_overrides): + def test_without_oauth_code(self, config_overrides: Callable[..., None]) -> None: config_overrides( CREATORS_PLATFORM_API_URL="https://creators.example.com", ) @@ -99,7 +25,7 @@ class TestGetRedirectUrl: assert "oauth_code" not in url assert url.startswith("https://creators.example.com") - def test_with_oauth_code(self, config_overrides): + def test_with_oauth_code(self, config_overrides: Callable[..., None]) -> None: config_overrides( CREATORS_PLATFORM_API_URL="https://creators.example.com", ) @@ -111,7 +37,7 @@ class TestGetRedirectUrl: assert "dsl_claim_code=claim-abc" in url assert "oauth_code=oauth-code-123" in url - def test_strips_trailing_slash(self, config_overrides): + def test_strips_trailing_slash(self, config_overrides: Callable[..., None]) -> None: config_overrides( CREATORS_PLATFORM_API_URL="https://creators.example.com/", ) @@ -122,3 +48,72 @@ class TestGetRedirectUrl: assert url.startswith("https://creators.example.com?") assert "creators.example.com/?" not in url + + +@pytest.fixture(autouse=True) +def creators_url(config_overrides: Callable[..., None]) -> None: + config_overrides(CREATORS_PLATFORM_API_URL="https://creators.example.com/") + + +def test_upload_sends_binary_package_as_multipart(monkeypatch: pytest.MonkeyPatch) -> None: + source = BytesIO() + with ZipFile(source, "w", compression=ZIP_DEFLATED) as archive: + archive.writestr("manifest.yaml", "format: dify.app\nformat_version: 1\n") + archive.writestr("app.yaml", "app:\n mode: workflow\n") + payload = source.getvalue() + source.seek(0) + requests: list[httpx.Request] = [] + + def respond(request: httpx.Request) -> httpx.Response: + requests.append(request) + return httpx.Response(200, json={"data": {"claim_code": "claim"}}) + + with source, httpx.Client(transport=httpx.MockTransport(respond)) as client: + monkeypatch.setattr("core.helper.ssrf_proxy._get_ssrf_client", lambda _: client) + assert creators.upload_package(source, filename="example.ifpkg") == "claim" + assert not source.closed + + assert len(requests) == 1 + request = requests[0] + assert request.method == "POST" + assert str(request.url) == "https://creators.example.com/api/v1/templates/anonymous-upload" + assert request.headers["content-type"].startswith("multipart/form-data; boundary=") + assert b'name="file"; filename="example.ifpkg"' in request.content + assert b"Content-Type: application/octet-stream" in request.content + assert payload in request.content + assert request.extensions["timeout"] == {"connect": 5.0, "read": 30.0, "write": 30.0, "pool": 30.0} + + +@pytest.mark.parametrize("data", [None, {}, {"claim_code": ""}, {"claim_code": 123}]) +def test_upload_rejects_invalid_claim_code(data: object) -> None: + response = httpx.Response(200, request=httpx.Request("POST", "https://creators.example.com"), json={"data": data}) + with ( + BytesIO(b"package") as source, + patch("core.helper.creators.ssrf_proxy.post", return_value=response), + ): + with pytest.raises(ValueError, match="claim_code"): + creators.upload_package(source, filename="example.ifpkg") + assert not source.closed + + +@pytest.mark.parametrize("failure", ["http", "network"]) +def test_upload_propagates_failure_without_retry(failure: str) -> None: + attempts = 0 + + def respond(request: httpx.Request) -> httpx.Response: + nonlocal attempts + attempts += 1 + if failure == "network": + raise httpx.ConnectError("Connection failed", request=request) + return httpx.Response(500) + + with ( + BytesIO(b"package") as source, + httpx.Client(transport=httpx.MockTransport(respond)) as client, + patch("core.helper.ssrf_proxy._get_ssrf_client", return_value=client), + ): + expected_error = httpx.ConnectError if failure == "network" else httpx.HTTPStatusError + with pytest.raises(expected_error): + creators.upload_package(source, filename="example.ifpkg") + assert not source.closed + assert attempts == 1 diff --git a/api/tests/unit_tests/services/app/test_console_service.py b/api/tests/unit_tests/services/app/test_console_service.py index 6455fbb61f0..e5e17582ef3 100644 --- a/api/tests/unit_tests/services/app/test_console_service.py +++ b/api/tests/unit_tests/services/app/test_console_service.py @@ -226,18 +226,26 @@ class Transfers: @dataclass class Creators: enabled: bool = True + failure: Literal["upload", "authorize"] | None = None + uploads: list[BinaryIO] = field(default_factory=list) calls: list[tuple[object, ...]] = field(default_factory=list) def require_enabled(self) -> None: if not self.enabled: raise CreatorsPlatformDisabledError - def upload(self, dsl: str) -> str: - self.calls.append(("upload", dsl)) + def upload(self, source: BinaryIO, *, filename: str) -> str: + self.uploads.append(source) + self.calls.append(("upload", filename, source.read())) + if self.failure == "upload": + raise RuntimeError("Upload failed") return "claim" def authorize(self, account_id: str) -> str | None: + assert all(source.closed for source in self.uploads) self.calls.append(("authorize", account_id)) + if self.failure == "authorize": + raise RuntimeError("Authorization failed") return "code" def redirect_url(self, claim_code: str, oauth_code: str | None) -> str: @@ -390,11 +398,42 @@ def test_export_rejects_invalid_or_unpaid_version_before_exporting(ports: Ports) service.export(CONTEXT, "app", AppExportOptions()) -def test_publish_orders_upload_authorization_and_redirect_without_secrets(ports: Ports) -> None: - service, _, _, transfers, creators = ports +@pytest.mark.parametrize("mode", ["chat", "workflow", "agent"]) +def test_publish_orders_package_upload_authorization_and_redirect_without_secrets(ports: Ports, mode: str) -> None: + service, apps, _, transfers, creators = ports + apps.source = replace(apps.source, mode=mode, bound_agent_id="agent" if mode == "agent" else None) assert service.publish(CONTEXT, "app") == "https://creators.example.com" - assert transfers.calls == [("dsl", CONTEXT, "app", AppExportOptions(include_secret=False))] - assert creators.calls == [("upload", "app: example"), ("authorize", "actor"), ("redirect", "claim", "code")] + if mode == "agent": + assert transfers.calls == [("agent", "workspace", "agent", None)] + filename, payload = "agent.ifpkg", b"agent" + else: + assert transfers.calls == [("package", CONTEXT, "app", AppExportOptions(format="ifpkg", include_secret=False))] + filename, payload = "example.ifpkg", b"package" + assert creators.calls == [("upload", filename, payload), ("authorize", "actor"), ("redirect", "claim", "code")] + assert len(creators.uploads) == 1 + assert creators.uploads[0].closed + assert apps.calls == [("get_reference", CONTEXT, "app")] + + +@pytest.mark.parametrize("failure", ["upload", "authorize"]) +def test_publish_closes_package_and_stops_after_failure(ports: Ports, failure: Literal["upload", "authorize"]) -> None: + service, _, _, _, creators = ports + creators.failure = failure + with pytest.raises(RuntimeError, match="failed"): + service.publish(CONTEXT, "app") + assert creators.uploads[0].closed + expected: list[tuple[object, ...]] = [("upload", "example.ifpkg", b"package")] + if failure == "authorize": + expected.append(("authorize", "actor")) + assert creators.calls == expected + + +def test_publish_rejects_unbound_agent_before_upload(ports: Ports) -> None: + service, apps, _, transfers, creators = ports + apps.source = replace(apps.source, mode="agent") + with pytest.raises(ConsoleAppNotFoundError, match="Agent not found"): + service.publish(CONTEXT, "app") + assert transfers.calls == creators.calls == [] def test_disabled_creators_has_no_export_or_external_effects(ports: Ports) -> None: diff --git a/api/tests/unit_tests/services/app/test_creators_platform_gateway.py b/api/tests/unit_tests/services/app/test_creators_platform_gateway.py index e115f0586fa..13ba354b405 100644 --- a/api/tests/unit_tests/services/app/test_creators_platform_gateway.py +++ b/api/tests/unit_tests/services/app/test_creators_platform_gateway.py @@ -2,13 +2,16 @@ from collections.abc import Callable from dataclasses import dataclass, field +from io import BytesIO from typing import cast +from unittest.mock import patch from urllib.parse import parse_qs, urlsplit import pytest from services.app import creators_platform_gateway as gateway_module from services.app.console_service import CreatorsPlatformDisabledError +from services.app.creators_platform_gateway import CreatorsPlatformGateway from services.entities.oauth_server_entities import OAuthAuthorizationCode from services.oauth_server_service import OAuthServerService @@ -51,3 +54,11 @@ def test_gateway_rejects_disabled_feature(config_overrides: Callable[..., None]) gateway = gateway_module.CreatorsPlatformGateway(oauth=cast(OAuthServerService, OAuth())) with pytest.raises(CreatorsPlatformDisabledError): gateway.require_enabled() + + +def test_gateway_forwards_package_stream_and_filename() -> None: + gateway = CreatorsPlatformGateway(oauth=cast(OAuthServerService, OAuth())) + with BytesIO(b"package") as source, patch("core.helper.creators.upload_package", return_value="claim") as upload: + assert gateway.upload(source, filename="example.ifpkg") == "claim" + upload.assert_called_once_with(source, filename="example.ifpkg") + assert not source.closed