opensource/dify
2
0
Fork 0
mirror of https://github.com/langgenius/dify.git synced 2026-09-08 11:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(api): reject ownerless pending DSL imports (#40107)

Browse Source
This commit is contained in:
WH-2099 2026-09-07 10:59:09 +00:00 committed by GitHub
parent 715bc7cd95
commit a9cb17eb6c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 120 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
api/services/app_dsl_service.py
View File

@ -244,8 +244,11 @@ class AppDslService:
# If major version mismatch, store import info in Redis
if status == ImportStatus.PENDING:
tenant_id = account.current_tenant_id
if tenant_id is None:
raise ValueError("Current tenant is not set")
pending_data = PendingData(
tenant_id=account.current_tenant_id,
tenant_id=tenant_id,
account_id=account.id,
import_mode=import_mode,
yaml_content=content,

14
api/services/entities/dsl_entities.py
View File

@ -1,7 +1,7 @@
from enum import StrEnum
from typing import Any
from pydantic import BaseModel, Field
from pydantic import BaseModel, ConfigDict, Field
from core.plugin.entities.plugin import PluginDependency
@ -19,15 +19,13 @@ class ImportStatus(StrEnum):
class PendingImportOwner(BaseModel):
tenant_id: str | None = None
account_id: str | None = None
model_config = ConfigDict(hide_input_in_errors=True)
tenant_id: str
account_id: str
def is_accessible_by(self, *, tenant_id: str | None, account_id: str) -> bool:
if tenant_id is None:
return False
owner = (self.tenant_id, self.account_id)
# Ownerless payloads come from older pods and expire after 10 minutes; #40106 removes this bridge.
return owner in ((None, None), (tenant_id, account_id))
return tenant_id is not None and (self.tenant_id, self.account_id) == (tenant_id, account_id)
class DslImportWarning(BaseModel):

5
api/services/rag_pipeline/rag_pipeline_dsl_service.py
View File

@ -221,8 +221,11 @@ class RagPipelineDslService:
# If major version mismatch, store import info in Redis
if status == ImportStatus.PENDING:
tenant_id = account.current_tenant_id
if tenant_id is None:
raise ValueError("Current tenant is not set")
pending_data = RagPipelinePendingData(
tenant_id=account.current_tenant_id,
tenant_id=tenant_id,
account_id=account.id,
import_mode=import_mode,
yaml_content=content,

5
api/services/snippet_dsl_service.py
View File

@ -235,8 +235,11 @@ class SnippetDslService:
# If major version mismatch, store import info in Redis
if status == ImportStatus.PENDING:
tenant_id = account.current_tenant_id
if tenant_id is None:
raise ValueError("Current tenant is not set")
pending_data = SnippetPendingData(
tenant_id=account.current_tenant_id,
tenant_id=tenant_id,
account_id=account.id,
import_mode=import_mode,
yaml_content=content,

8
api/tests/test_containers_integration_tests/services/test_app_dsl_service.py
View File

@ -600,6 +600,8 @@ class TestAppDslService:
redis_key = f"{IMPORT_INFO_REDIS_KEY_PREFIX}{import_id}"
pending = PendingData(
tenant_id=_DEFAULT_TENANT_ID,
account_id=_DEFAULT_ACCOUNT_ID,
import_mode=ImportMode.YAML_CONTENT,
yaml_content=_workflow_yaml(),
name="name",
@ -609,11 +611,7 @@ class TestAppDslService:
icon_background="#fff",
app_id=None,
)
redis_client.setex(
redis_key,
IMPORT_INFO_REDIS_EXPIRY,
pending.model_dump_json(exclude={"tenant_id", "account_id"}),
)
redis_client.setex(redis_key, IMPORT_INFO_REDIS_EXPIRY, pending.model_dump_json())
created_app = SimpleNamespace(
id=str(uuid4()),

6
api/tests/unit_tests/controllers/console/app/test_app_import_api.py
View File

@ -343,7 +343,8 @@ class TestAppImportConfirmApi:
)
redis_get = MagicMock(
return_value=(
b'{"import_mode":"yaml-content","yaml_content":"app: {}","app_id":null,'
b'{"tenant_id":"tenant-1","account_id":"u1","import_mode":"yaml-content",'
b'"yaml_content":"app: {}","app_id":null,'
b'"name":null,"description":null,"icon_type":null,"icon":null,"icon_background":null}'
)
)
@ -388,7 +389,8 @@ class TestAppImportConfirmApi:
app_import_module.redis_client,
"get",
lambda *_args, **_kwargs: (
b'{"import_mode":"yaml-content","yaml_content":"app: {}","app_id":"existing-app",'
b'{"tenant_id":"tenant-1","account_id":"u1","import_mode":"yaml-content",'
b'"yaml_content":"app: {}","app_id":"existing-app",'
b'"name":null,"description":null,"icon_type":null,"icon":null,"icon_background":null}'
),
)

24
api/tests/unit_tests/services/rag_pipeline/test_rag_pipeline_dsl_service.py
View File

@ -643,6 +643,15 @@ def test_confirm_import_rejects_non_serialized_pending_data(
def test_import_pending_version_stores_redis(monkeypatch: pytest.MonkeyPatch, service: RagPipelineDslService) -> None:
setex = Mock()
monkeypatch.setattr(module.redis_client, "setex", setex)
missing_tenant = service.import_rag_pipeline(
account=Mock(id="account-1", current_tenant_id=None),
import_mode=ImportMode.YAML_CONTENT.value,
yaml_content=_valid_dsl(version="1.0.0"),
)
assert missing_tenant.status == ImportStatus.FAILED
assert missing_tenant.error == "Current tenant is not set"
setex.assert_not_called()
result = service.import_rag_pipeline(
account=_account(), import_mode=ImportMode.YAML_CONTENT.value, yaml_content=_valid_dsl(version="1.0.0")
)
@ -813,14 +822,27 @@ def test_confirm_import_updates_tenant_pipeline_and_dataset(
pipeline_id=pipeline.id,
)
redis_key = "app_import_info:import-1"
pending_json = pending.model_dump_json(exclude={"tenant_id", "account_id"})
monkeypatch.setattr(
module.redis_client,
"get",
Mock(side_effect=lambda key: pending.model_dump_json() if key == redis_key else None),
Mock(side_effect=lambda key: pending_json if key == redis_key else None),
)
delete = Mock()
monkeypatch.setattr(module.redis_client, "delete", delete)
load = Mock(wraps=module.yaml.safe_load)
monkeypatch.setattr(module.yaml, "safe_load", load)
create_or_update = Mock(wraps=service._create_or_update_pipeline)
monkeypatch.setattr(service, "_create_or_update_pipeline", create_or_update)
monkeypatch.setattr(module.KnowledgeConfiguration, "model_validate", Mock(return_value=_knowledge_configuration()))
assert service.confirm_import(import_id="import-1", account=_account()).status == ImportStatus.FAILED
load.assert_not_called()
create_or_update.assert_not_called()
delete.assert_not_called()
assert pipeline.name == "Pipeline"
pending_json = pending.model_dump_json()
for foreign_account in (_account(tenant_id="tenant-2"), _account(account_id="account-2")):
assert service.confirm_import(import_id="import-1", account=foreign_account).status == ImportStatus.FAILED
delete.assert_not_called()

62
api/tests/unit_tests/services/test_app_dsl_service.py
View File

@ -348,11 +348,17 @@ def test_pending_import_is_scoped_to_its_owner(monkeypatch: pytest.MonkeyPatch,
monkeypatch.setattr("services.app_dsl_service.redis_client.get", pending_imports.get)
monkeypatch.setattr("services.app_dsl_service.redis_client.delete", pending_imports.pop)
monkeypatch.setattr(
service,
"_create_or_update_app",
Mock(return_value=_app(app_id="app-1", mode=AppMode.WORKFLOW)),
)
create_or_update = Mock(return_value=_app(app_id="app-1", mode=AppMode.WORKFLOW))
monkeypatch.setattr(service, "_create_or_update_app", create_or_update)
load = Mock(wraps=yaml.safe_load)
monkeypatch.setattr("services.app_dsl_service.yaml.safe_load", load)
pending_imports[redis_key] = pending_data.model_dump_json(exclude={"tenant_id", "account_id"})
assert service.confirm_import(import_id=pending.id, account=creator).status == ImportStatus.FAILED
load.assert_not_called()
create_or_update.assert_not_called()
assert redis_key in pending_imports
pending_imports[redis_key] = pending_data.model_dump_json()
for other_account in (
_account(tenant_id="tenant-2"),
@ -364,36 +370,60 @@ def test_pending_import_is_scoped_to_its_owner(monkeypatch: pytest.MonkeyPatch,
assert redis_key not in pending_imports
def test_pending_import_requires_current_tenant(monkeypatch: pytest.MonkeyPatch, unbound_session: Session) -> None:
setex = Mock()
monkeypatch.setattr("services.app_dsl_service.redis_client.setex", setex)
account = _account()
account._current_tenant = None
result = AppDslService(session=unbound_session).import_app(
account=account,
import_mode="yaml-content",
yaml_content="version: 99.0.0\nkind: app\napp: {name: Test, mode: workflow}\n",
)
assert result.status == ImportStatus.FAILED
assert result.error == "Current tenant is not set"
setex.assert_not_called()
@pytest.mark.parametrize(
("tenant_id", "account_id", "expected"),
("caller_tenant_id", "caller_account_id", "expected"),
[
("tenant-1", "account-1", True),
(None, "account-1", False),
("tenant-1", None, False),
("tenant-2", "account-1", False),
("tenant-1", "account-2", False),
],
)
def test_pending_import_owner_access(
tenant_id: str | None,
account_id: str | None,
caller_tenant_id: str | None,
caller_account_id: str,
expected: bool,
) -> None:
pending = PendingData(
tenant_id=tenant_id,
account_id=account_id,
tenant_id="tenant-1",
account_id="account-1",
import_mode="yaml-content",
yaml_content="",
)
assert pending.is_accessible_by(tenant_id="tenant-1", account_id="account-1") is expected
assert pending.is_accessible_by(tenant_id=caller_tenant_id, account_id=caller_account_id) is expected
def test_pending_import_owner_access_accepts_legacy_json() -> None:
pending = PendingData.model_validate_json('{"import_mode":"yaml-content","yaml_content":""}')
@pytest.mark.parametrize(
"payload",
[
'{"import_mode":"yaml-content","yaml_content":"secret-token-123"}',
'{"tenant_id":"tenant-1","import_mode":"yaml-content","yaml_content":"secret-token-123"}',
'{"account_id":"account-1","import_mode":"yaml-content","yaml_content":"secret-token-123"}',
],
)
def test_pending_import_owner_is_required(payload: str) -> None:
with pytest.raises(ValueError) as exc_info:
PendingData.model_validate_json(payload)
assert pending.is_accessible_by(tenant_id="tenant-1", account_id="account-1")
assert not pending.is_accessible_by(tenant_id=None, account_id="account-1")
assert "secret-token-123" not in str(exc_info.value)
def test_create_or_update_app_loads_existing_model_config_with_service_session(

27
api/tests/unit_tests/services/test_snippet_dsl_service.py
View File

@ -3,6 +3,7 @@ from types import SimpleNamespace
from unittest.mock import Mock
import pytest
import yaml
from sqlalchemy import event
from sqlalchemy.orm import Session
@ -318,6 +319,17 @@ workflow:
edges: []
"""
account_without_tenant = _account()
account_without_tenant._current_tenant = None
missing_tenant = service.import_snippet(
account=account_without_tenant,
import_mode=ImportMode.YAML_CONTENT.value,
yaml_content=yaml_content,
)
assert missing_tenant.status == ImportStatus.FAILED
assert missing_tenant.error == "Current tenant is not set"
setex.assert_not_called()
result = service.import_snippet(
account=_account(),
import_mode=ImportMode.YAML_CONTENT.value,
@ -459,12 +471,21 @@ workflow:
create_or_update = Mock(return_value=snippet)
monkeypatch.setattr(service, "_create_or_update_snippet", create_or_update)
redis_key = "snippet_import_info:import-1"
pending_json = pending.model_dump_json(exclude={"tenant_id", "account_id"})
monkeypatch.setattr(
"services.snippet_dsl_service.redis_client.get",
Mock(side_effect=lambda key: pending.model_dump_json() if key == redis_key else None),
Mock(side_effect=lambda key: pending_json if key == redis_key else None),
)
redis_delete = Mock()
monkeypatch.setattr("services.snippet_dsl_service.redis_client.delete", redis_delete)
load = Mock(wraps=yaml.safe_load)
monkeypatch.setattr("services.snippet_dsl_service.yaml.safe_load", load)
assert service.confirm_import(import_id="import-1", account=account).status == ImportStatus.FAILED
load.assert_not_called()
create_or_update.assert_not_called()
redis_delete.assert_not_called()
pending_json = pending.model_dump_json()
for other_account in (
_account(tenant_id="tenant-2"),
@ -491,6 +512,8 @@ def test_confirm_import_returns_failed_for_non_mapping_yaml(
service: SnippetDslService, monkeypatch: pytest.MonkeyPatch
):
pending = SnippetPendingData(
tenant_id="tenant-1",
account_id="account-1",
import_mode="yaml-content",
yaml_content="- item",
snippet_id=None,
@ -509,6 +532,8 @@ def test_confirm_import_returns_failed_when_create_or_update_raises(
rollback_events: list[str] = []
event.listen(sqlite_session, "after_rollback", lambda _session: rollback_events.append("rollback"))
pending = SnippetPendingData(
tenant_id="tenant-1",
account_id="account-1",
import_mode="yaml-content",
yaml_content="version: 0.1.0\nkind: snippet\nsnippet:\n name: Bad\n",
snippet_id="snippet-1",