mirror of
https://github.com/langgenius/dify.git
synced 2026-09-28 06:13:22 +08:00
fix(agent): allow package imports with missing plugins (#42913)
This commit is contained in:
@@ -1,19 +1,16 @@
|
||||
from http import HTTPStatus
|
||||
from typing import BinaryIO, Literal, cast
|
||||
from typing import BinaryIO, cast
|
||||
|
||||
from flask import request
|
||||
from flask_restx import Resource
|
||||
from pydantic import Field
|
||||
from werkzeug.exceptions import Forbidden
|
||||
|
||||
from controllers.common.rbac import PlainApp, RBACCheck, Workspace
|
||||
from controllers.common.schema import register_enum_models, register_response_schema_models, register_schema_models
|
||||
from controllers.common.schema import register_enum_models, register_schema_models
|
||||
from controllers.console.app.error import AppNotFoundError
|
||||
from controllers.console.flask_admission import console_account_admission
|
||||
from controllers.console.wraps import RBACPermission, validate_request
|
||||
from core.plugin.entities.plugin import PluginDependency
|
||||
from extensions.ext_application_services import application_services
|
||||
from fields.base import ResponseModel
|
||||
from machinery.context import RequestContext
|
||||
from models.account import TenantAccountRole
|
||||
from services.agent.errors import InvalidRosterAgentPackageError
|
||||
@@ -28,16 +25,8 @@ class AppImportPayload(AppImportParams):
|
||||
pass
|
||||
|
||||
|
||||
class RosterAgentPackageConflictResponse(ResponseModel):
|
||||
code: str
|
||||
message: str
|
||||
status: Literal[HTTPStatus.CONFLICT] = HTTPStatus.CONFLICT
|
||||
leaked_dependencies: list[PluginDependency] = Field(default_factory=list)
|
||||
|
||||
|
||||
register_enum_models(console_ns, ImportStatus)
|
||||
register_schema_models(console_ns, AppImportPayload, Import, CheckDependenciesResult)
|
||||
register_response_schema_models(console_ns, RosterAgentPackageConflictResponse)
|
||||
|
||||
|
||||
_EDIT_ROLES = frozenset({TenantAccountRole.OWNER, TenantAccountRole.ADMIN, TenantAccountRole.EDITOR})
|
||||
@@ -77,12 +66,8 @@ class AppImportApi(Resource):
|
||||
@console_ns.response(HTTPStatus.OK, "Import completed", console_ns.models[Import.__name__])
|
||||
@console_ns.response(HTTPStatus.ACCEPTED, "Import pending confirmation", console_ns.models[Import.__name__])
|
||||
@console_ns.response(HTTPStatus.BAD_REQUEST, "Import failed", console_ns.models[Import.__name__])
|
||||
@console_ns.response(HTTPStatus.FORBIDDEN, "Insufficient import or plugin installation permissions")
|
||||
@console_ns.response(
|
||||
HTTPStatus.CONFLICT,
|
||||
"Agent name conflict or missing plugins",
|
||||
console_ns.models[RosterAgentPackageConflictResponse.__name__],
|
||||
)
|
||||
@console_ns.response(HTTPStatus.FORBIDDEN, "Insufficient import permissions")
|
||||
@console_ns.response(HTTPStatus.CONFLICT, "Agent name conflict")
|
||||
@console_ns.response(HTTPStatus.REQUEST_ENTITY_TOO_LARGE, "Roster Agent package exceeds the size limit")
|
||||
@console_account_admission(allowed_roles=_EDIT_ROLES)
|
||||
def post(self, context: RequestContext):
|
||||
|
||||
@@ -1549,8 +1549,8 @@ Create a new application
|
||||
| 200 | Import completed | **application/json**: [Import](#import)<br> |
|
||||
| 202 | Import pending confirmation | **application/json**: [Import](#import)<br> |
|
||||
| 400 | Import failed | **application/json**: [Import](#import)<br> |
|
||||
| 403 | Insufficient import or plugin installation permissions | |
|
||||
| 409 | Agent name conflict or missing plugins | **application/json**: [RosterAgentPackageConflictResponse](#rosteragentpackageconflictresponse)<br> |
|
||||
| 403 | Insufficient import permissions | |
|
||||
| 409 | Agent name conflict | |
|
||||
| 413 | Roster Agent package exceeds the size limit | |
|
||||
|
||||
### [GET] /apps/imports/{app_id}/check-dependencies
|
||||
@@ -22276,15 +22276,6 @@ Resource types understood by access policies.
|
||||
| ---- | ---- | ----------- | -------- |
|
||||
| data | [ [AccessPolicyRoleBinding](#accesspolicyrolebinding) ] | | No |
|
||||
|
||||
#### RosterAgentPackageConflictResponse
|
||||
|
||||
| Name | Type | Description | Required |
|
||||
| ---- | ---- | ----------- | -------- |
|
||||
| code | string | | Yes |
|
||||
| leaked_dependencies | [ [PluginDependency](#plugindependency) ] | | No |
|
||||
| message | string | | Yes |
|
||||
| status | integer, <br>**Default:** 409 | | No |
|
||||
|
||||
#### RosterListQuery
|
||||
|
||||
| Name | Type | Description | Required |
|
||||
|
||||
@@ -77,12 +77,6 @@ class RosterAgentPackageImportFailedError(BaseHTTPException):
|
||||
code = 500
|
||||
|
||||
|
||||
class RosterAgentPackageDependenciesMissingError(BaseHTTPException):
|
||||
error_code = "roster_agent_package_dependencies_missing"
|
||||
description = "Install the missing plugins before importing the Roster Agent package."
|
||||
code = 409
|
||||
|
||||
|
||||
class RosterAgentPackageResourceUnavailableError(BaseHTTPException):
|
||||
error_code = "roster_agent_package_resource_unavailable"
|
||||
description = "The Roster Agent package resources could not be stored."
|
||||
|
||||
@@ -1,40 +0,0 @@
|
||||
"""Preflight plugin availability and installation permission for package imports."""
|
||||
|
||||
from werkzeug.exceptions import Forbidden
|
||||
|
||||
from configs import dify_config
|
||||
from core.db.session_factory import session_factory
|
||||
from core.plugin.entities.plugin import PluginDependency
|
||||
from core.rbac import RBACPermission
|
||||
from models import Account
|
||||
from models.account import TenantPluginInstallPermission
|
||||
from services.agent.errors import RosterAgentPackageDependenciesMissingError
|
||||
from services.enterprise.rbac_service import RBACService
|
||||
from services.plugin.dependencies_analysis import DependenciesAnalysisService
|
||||
from services.plugin.plugin_permission_service import PluginPermissionService
|
||||
|
||||
|
||||
def check_package_dependencies(*, tenant_id: str, account: Account, dependencies: list[PluginDependency]) -> None:
|
||||
if not dependencies:
|
||||
return
|
||||
missing = DependenciesAnalysisService.get_leaked_dependencies(tenant_id=tenant_id, dependencies=dependencies)
|
||||
if not missing:
|
||||
return
|
||||
if dify_config.RBAC_ENABLED:
|
||||
allowed = RBACService.CheckAccess.check(
|
||||
tenant_id, account.id, scene=RBACPermission.PLUGIN_INSTALL, resource_type=None, resource_id=None
|
||||
)
|
||||
if not allowed:
|
||||
raise Forbidden("Plugin installation permission is required to import this Agent package")
|
||||
else:
|
||||
with session_factory.create_session() as session:
|
||||
permission = PluginPermissionService.get_permission(tenant_id, session=session)
|
||||
install_permission = permission.install_permission if permission is not None else None
|
||||
if install_permission == TenantPluginInstallPermission.NOBODY or (
|
||||
install_permission == TenantPluginInstallPermission.ADMINS and not account.is_admin_or_owner
|
||||
):
|
||||
raise Forbidden("Plugin installation permission is required to import this Agent package")
|
||||
error = RosterAgentPackageDependenciesMissingError()
|
||||
assert error.data is not None
|
||||
error.data["leaked_dependencies"] = [item.model_dump(mode="json") for item in missing]
|
||||
raise error
|
||||
@@ -35,10 +35,10 @@ from services.agent.errors import (
|
||||
RosterAgentPackageTooLargeError,
|
||||
)
|
||||
from services.agent.package_resource_importer import AgentPackageResourceImporter, _Storage
|
||||
from services.agent.roster_package_dependencies import check_package_dependencies
|
||||
from services.agent.roster_package_reader import RosterAgentPackageReader
|
||||
from services.agent.roster_service import AgentRosterService
|
||||
from services.app_creation_records import create_installed_app_record, create_site_record
|
||||
from services.app_dsl_service import AppDslService
|
||||
from services.app_service import AppService
|
||||
from services.entities.dsl_entities import DslImportWarning
|
||||
from services.icon_configuration import DEFAULT_ICON, DEFAULT_ICON_BACKGROUND, DEFAULT_ICON_TYPE, is_valid_image_icon
|
||||
@@ -73,7 +73,6 @@ class RosterAgentPackageImporter:
|
||||
agent_package = app_dsl.package
|
||||
self._resources.validate(resources=package.manifest, agent_package=agent_package)
|
||||
|
||||
check_package_dependencies(tenant_id=tenant_id, account=account, dependencies=app_dsl.dependencies)
|
||||
try:
|
||||
icons = self._resources.materialize_icons(
|
||||
archive=package, icons=package.manifest.icons, tenant_id=tenant_id, account_id=account.id
|
||||
@@ -133,6 +132,16 @@ class RosterAgentPackageImporter:
|
||||
app_id,
|
||||
exc_info=True,
|
||||
)
|
||||
if app_dsl.dependencies:
|
||||
try:
|
||||
AppDslService.cache_import_dependencies(app_id=app_id, dependencies=app_dsl.dependencies)
|
||||
except Exception:
|
||||
logger.warning(
|
||||
"Imported Agent App dependency check could not be cached: tenant_id=%s app_id=%s",
|
||||
tenant_id,
|
||||
app_id,
|
||||
exc_info=True,
|
||||
)
|
||||
return RosterAgentPackageImportResult(app_id=app_id, agent_id=agent_id, warnings=warnings)
|
||||
|
||||
@staticmethod
|
||||
|
||||
@@ -469,6 +469,14 @@ class AppDslService:
|
||||
leaked_dependencies=leaked_dependencies,
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def cache_import_dependencies(*, app_id: str, dependencies: list[PluginDependency]) -> None:
|
||||
redis_client.setex(
|
||||
f"{CHECK_DEPENDENCIES_REDIS_KEY_PREFIX}{app_id}",
|
||||
IMPORT_INFO_REDIS_EXPIRY,
|
||||
CheckDependenciesPendingData(app_id=app_id, dependencies=dependencies).model_dump_json(),
|
||||
)
|
||||
|
||||
def _load_app_for_overwrite(self, account: Account, app_id: str) -> App | None:
|
||||
if account.current_tenant_id is None:
|
||||
raise ValueError("Current tenant is not set")
|
||||
@@ -612,11 +620,7 @@ class AppDslService:
|
||||
|
||||
# save dependencies
|
||||
if dependencies:
|
||||
redis_client.setex(
|
||||
f"{CHECK_DEPENDENCIES_REDIS_KEY_PREFIX}{app.id}",
|
||||
IMPORT_INFO_REDIS_EXPIRY,
|
||||
CheckDependenciesPendingData(app_id=app.id, dependencies=dependencies).model_dump_json(),
|
||||
)
|
||||
self.cache_import_dependencies(app_id=app.id, dependencies=dependencies)
|
||||
|
||||
# Initialize app based on mode
|
||||
match app_mode:
|
||||
|
||||
@@ -692,9 +692,6 @@ def test_generate_specs_include_console_contract_shapes_for_schema_migration(tmp
|
||||
assert _request_schema(package_import, "multipart/form-data")["properties"]["app_id"]["type"] == "string"
|
||||
assert _request_schema(package_import, "application/json")["$ref"] == "#/components/schemas/AppImportPayload"
|
||||
assert "mode" in schemas["AppImportPayload"]["required"]
|
||||
conflict = package_import["responses"]["409"]["content"]["application/json"]["schema"]
|
||||
assert conflict["$ref"] == "#/components/schemas/RosterAgentPackageConflictResponse"
|
||||
assert "leaked_dependencies" in schemas["RosterAgentPackageConflictResponse"]["properties"]
|
||||
assert "403" in package_import["responses"]
|
||||
export = paths["/apps/{app_id}/export"]["get"]
|
||||
assert export["responses"]["200"]["content"]["application/zip"]["schema"] == {"type": "string", "format": "binary"}
|
||||
|
||||
@@ -4,19 +4,17 @@ import hashlib
|
||||
import io
|
||||
import json
|
||||
import zipfile
|
||||
from collections.abc import Callable, Generator
|
||||
from collections.abc import Generator
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
from sqlalchemy import event, func, select
|
||||
from sqlalchemy.exc import IntegrityError
|
||||
from sqlalchemy.orm import Session, sessionmaker
|
||||
from werkzeug.exceptions import Forbidden
|
||||
|
||||
from core.db.session_factory import session_factory
|
||||
from core.plugin.entities.plugin import PluginDependency, PluginDependencyType
|
||||
from models import Account
|
||||
from models.account import TenantPluginDebugPermission, TenantPluginInstallPermission, TenantPluginPermission
|
||||
from models.agent import (
|
||||
Agent,
|
||||
AgentConfigDraft,
|
||||
@@ -42,7 +40,6 @@ from services.agent.dsl_entities import (
|
||||
from services.agent.errors import (
|
||||
AgentNameConflictError,
|
||||
InvalidRosterAgentPackageError,
|
||||
RosterAgentPackageDependenciesMissingError,
|
||||
RosterAgentPackageImportFailedError,
|
||||
RosterAgentPackageResourceUnavailableError,
|
||||
RosterAgentPackageTooLargeError,
|
||||
@@ -56,6 +53,7 @@ from services.agent.roster_package_entities import (
|
||||
RosterAgentPackageSkill,
|
||||
)
|
||||
from services.agent.roster_package_importer import RosterAgentPackageImporter
|
||||
from services.app_dsl_service import AppDslService
|
||||
from services.app_service import AppService
|
||||
from services.file_service import FileService
|
||||
from services.plugin.dependencies_analysis import DependenciesAnalysisService
|
||||
@@ -83,8 +81,17 @@ class _MemoryStorage:
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _installed_plugins(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setattr(DependenciesAnalysisService, "get_leaked_dependencies", lambda **_kwargs: [])
|
||||
def _dependency_cache(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
cached: dict[str, str] = {}
|
||||
|
||||
class Cache:
|
||||
def setex(self, key: str, _expiry: int, value: str) -> None:
|
||||
cached[key] = value
|
||||
|
||||
def get(self, key: str) -> str | None:
|
||||
return cached.get(key)
|
||||
|
||||
monkeypatch.setattr("services.app_dsl_service.redis_client", Cache())
|
||||
|
||||
|
||||
def _account() -> Account:
|
||||
@@ -119,6 +126,7 @@ def _package(
|
||||
name: str = "Imported Agent",
|
||||
binary_dependency: bool = False,
|
||||
missing_knowledge: bool = False,
|
||||
include_dependency: bool = True,
|
||||
) -> bytes:
|
||||
config_skill = _skill_archive("config-skill")
|
||||
workspace_skill = _skill_archive("workspace-skill")
|
||||
@@ -189,7 +197,7 @@ def _package(
|
||||
],
|
||||
)
|
||||
},
|
||||
dependencies=[dependency],
|
||||
dependencies=[dependency] if include_dependency else [],
|
||||
)
|
||||
app_bytes = yaml.safe_dump(app.model_dump(mode="json")).encode()
|
||||
manifest = RosterAgentPackageManifest(
|
||||
@@ -426,58 +434,37 @@ def test_import_clears_source_credentials(
|
||||
assert "source-id" not in json.dumps(data)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("allowed", [False, True])
|
||||
def test_missing_plugins_are_checked_before_writes(
|
||||
monkeypatch: pytest.MonkeyPatch, config_overrides: Callable[..., None], allowed: bool
|
||||
) -> None:
|
||||
config_overrides(RBAC_ENABLED=True)
|
||||
monkeypatch.setattr(DependenciesAnalysisService, "get_leaked_dependencies", lambda **kwargs: kwargs["dependencies"])
|
||||
monkeypatch.setattr(
|
||||
"services.agent.roster_package_dependencies.RBACService.CheckAccess.check", lambda *_args, **_kwargs: allowed
|
||||
)
|
||||
storage = _MemoryStorage()
|
||||
with pytest.raises(RosterAgentPackageDependenciesMissingError if allowed else Forbidden) as failure:
|
||||
RosterAgentPackageImporter(storage_backend=storage).import_package(
|
||||
source=io.BytesIO(_package()), tenant_id="tenant-1", account=_account()
|
||||
)
|
||||
assert storage.save_count == 0
|
||||
if allowed:
|
||||
assert isinstance(failure.value, RosterAgentPackageDependenciesMissingError)
|
||||
assert failure.value.data is not None
|
||||
assert len(failure.value.data["leaked_dependencies"]) == 1
|
||||
|
||||
|
||||
def test_empty_dependencies_do_not_require_plugin_service(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
from services.agent.roster_package_dependencies import check_package_dependencies
|
||||
|
||||
def unavailable(**_kwargs) -> None:
|
||||
raise OSError("plugin service unavailable")
|
||||
|
||||
monkeypatch.setattr(DependenciesAnalysisService, "get_leaked_dependencies", unavailable)
|
||||
check_package_dependencies(tenant_id="tenant-1", account=_account(), dependencies=[])
|
||||
|
||||
|
||||
@pytest.mark.parametrize("policy", [TenantPluginInstallPermission.NOBODY, TenantPluginInstallPermission.ADMINS])
|
||||
def test_missing_plugins_respect_workspace_install_policy(
|
||||
@pytest.mark.parametrize("include_dependency", [False, True])
|
||||
def test_plugin_dependencies_do_not_block_import(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
config_overrides: Callable[..., None],
|
||||
sqlite_session_factory: sessionmaker[Session],
|
||||
policy: TenantPluginInstallPermission,
|
||||
include_dependency: bool,
|
||||
) -> None:
|
||||
config_overrides(RBAC_ENABLED=False)
|
||||
with sqlite_session_factory() as session, session.begin():
|
||||
session.add(
|
||||
TenantPluginPermission(
|
||||
tenant_id="tenant-1", install_permission=policy, debug_permission=TenantPluginDebugPermission.NOBODY
|
||||
)
|
||||
)
|
||||
monkeypatch.setattr(DependenciesAnalysisService, "get_leaked_dependencies", lambda **kwargs: kwargs["dependencies"])
|
||||
storage = _MemoryStorage()
|
||||
with pytest.raises(Forbidden):
|
||||
RosterAgentPackageImporter(storage_backend=storage).import_package(
|
||||
source=io.BytesIO(_package()), tenant_id="tenant-1", account=_account()
|
||||
)
|
||||
assert storage.save_count == 0
|
||||
result = RosterAgentPackageImporter(storage_backend=storage).import_package(
|
||||
source=io.BytesIO(_package(include_dependency=include_dependency)), tenant_id="tenant-1", account=_account()
|
||||
)
|
||||
assert storage.save_count > 0
|
||||
with sqlite_session_factory() as session:
|
||||
assert session.get(App, result.app_id) is not None
|
||||
missing = AppDslService.check_app_dependencies(tenant_id="tenant-1", app_id=result.app_id)
|
||||
assert len(missing.leaked_dependencies) == int(include_dependency)
|
||||
|
||||
|
||||
def test_dependency_cache_failure_does_not_fail_committed_import(
|
||||
monkeypatch: pytest.MonkeyPatch, sqlite_session_factory: sessionmaker[Session]
|
||||
) -> None:
|
||||
class UnavailableCache:
|
||||
def setex(self, *_args: object) -> None:
|
||||
raise OSError("cache unavailable")
|
||||
|
||||
monkeypatch.setattr("services.app_dsl_service.redis_client", UnavailableCache())
|
||||
result = RosterAgentPackageImporter(storage_backend=_MemoryStorage()).import_package(
|
||||
source=io.BytesIO(_package()), tenant_id="tenant-1", account=_account()
|
||||
)
|
||||
with sqlite_session_factory() as session:
|
||||
assert session.get(App, result.app_id) is not None
|
||||
|
||||
|
||||
def test_import_materializes_agent_resources_and_unpublished_draft(
|
||||
|
||||
@@ -76,13 +76,6 @@ export type Import = {
|
||||
warnings?: Array<DslImportWarning>
|
||||
}
|
||||
|
||||
export type RosterAgentPackageConflictResponse = {
|
||||
code: string
|
||||
leaked_dependencies?: Array<PluginDependency>
|
||||
message: string
|
||||
status?: 409
|
||||
}
|
||||
|
||||
export type CheckDependenciesResult = {
|
||||
leaked_dependencies?: Array<PluginDependency>
|
||||
}
|
||||
@@ -3576,7 +3569,7 @@ export type PostAppsImportsData = {
|
||||
export type PostAppsImportsErrors = {
|
||||
400: Import
|
||||
403: unknown
|
||||
409: RosterAgentPackageConflictResponse
|
||||
409: unknown
|
||||
413: unknown
|
||||
}
|
||||
|
||||
|
||||
@@ -2170,16 +2170,6 @@ export const zPluginDependency = z.object({
|
||||
value: z.union([zGithub, zMarketplace, zPackage]),
|
||||
})
|
||||
|
||||
/**
|
||||
* RosterAgentPackageConflictResponse
|
||||
*/
|
||||
export const zRosterAgentPackageConflictResponse = z.object({
|
||||
code: z.string(),
|
||||
leaked_dependencies: z.array(zPluginDependency).optional(),
|
||||
message: z.string(),
|
||||
status: z.literal(409).optional().default(409),
|
||||
})
|
||||
|
||||
/**
|
||||
* CheckDependenciesResult
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user