mirror of
https://github.com/langgenius/dify.git
synced 2026-05-05 18:08:07 +08:00
fix: namespace sandbox temp paths by sandbox ID to prevent cross-session collisions
DifyCli and AppAssets used hardcoded absolute paths under /tmp/.dify/,
causing concurrent sandbox sessions on the same host (e.g. SSH provider)
to overwrite each other's config files and CLI binaries.
- Add Sandbox.id property (uuid4 hex) as a stable, path-safe identifier
independent of provider-specific environment IDs
- Convert DifyCli/AppAssets from static constants to per-sandbox instances
namespaced under /tmp/.dify/{sandbox.id}/
- Replace all vm.metadata.id references with sandbox.id
- Replace upload_file with heredoc-based pipeline step in session.py to
reduce round-trips
This commit is contained in:
@ -47,15 +47,16 @@ class SandboxBashSession:
|
||||
def __enter__(self) -> SandboxBashSession:
|
||||
# Ensure sandbox initialization completes before any bash commands run.
|
||||
self._sandbox.wait_ready(timeout=SANDBOX_READY_TIMEOUT)
|
||||
cli = DifyCli(self._sandbox.id)
|
||||
self._cli_api_session = CliApiSessionManager().create(
|
||||
tenant_id=self._tenant_id,
|
||||
user_id=self._user_id,
|
||||
context=CliContext(tool_access=ToolAccessPolicy.from_dependencies(self._tools)),
|
||||
)
|
||||
if self._tools is not None and not self._tools.is_empty():
|
||||
tools_path = self._setup_node_tools_directory(self._node_id, self._tools, self._cli_api_session)
|
||||
tools_path = self._setup_node_tools_directory(cli, self._node_id, self._tools, self._cli_api_session)
|
||||
else:
|
||||
tools_path = DifyCli.GLOBAL_TOOLS_PATH
|
||||
tools_path = cli.global_tools_path
|
||||
|
||||
self._bash_tool = SandboxBashTool(
|
||||
sandbox=self._sandbox.vm,
|
||||
@ -66,11 +67,12 @@ class SandboxBashSession:
|
||||
|
||||
def _setup_node_tools_directory(
|
||||
self,
|
||||
cli: DifyCli,
|
||||
node_id: str,
|
||||
tools: ToolDependencies,
|
||||
cli_api_session: CliApiSession,
|
||||
) -> str:
|
||||
node_tools_path = f"{DifyCli.TOOLS_ROOT}/{node_id}"
|
||||
node_tools_path = f"{cli.tools_root}/{node_id}"
|
||||
config_json = json.dumps(
|
||||
DifyCliConfig.create(session=cli_api_session, tenant_id=self._tenant_id, tool_deps=tools).model_dump(
|
||||
mode="json"
|
||||
@ -83,7 +85,7 @@ class SandboxBashSession:
|
||||
# Merge mkdir + config write into a single pipeline to reduce round-trips.
|
||||
(
|
||||
pipeline(vm)
|
||||
.add(["mkdir", "-p", DifyCli.GLOBAL_TOOLS_PATH], error_message="Failed to create global tools dir")
|
||||
.add(["mkdir", "-p", cli.global_tools_path], error_message="Failed to create global tools dir")
|
||||
.add(["mkdir", "-p", node_tools_path], error_message="Failed to create node tools dir")
|
||||
# Use a quoted heredoc (<<'EOF') so the shell performs no expansion on the
|
||||
# content — safe regardless of $, `, \, or quotes inside the JSON.
|
||||
@ -95,7 +97,7 @@ class SandboxBashSession:
|
||||
)
|
||||
|
||||
pipeline(vm, cwd=node_tools_path).add(
|
||||
[DifyCli.PATH, "init"], error_message="Failed to initialize Dify CLI"
|
||||
[cli.bin_path, "init"], error_message="Failed to initialize Dify CLI"
|
||||
).execute(raise_on_error=True)
|
||||
|
||||
logger.info(
|
||||
|
||||
@ -2,18 +2,45 @@ from typing import Final
|
||||
|
||||
|
||||
class DifyCli:
|
||||
"""Dify CLI constants (absolute path - hidden in /tmp, not in sandbox workdir)"""
|
||||
"""Per-sandbox Dify CLI paths, namespaced under ``/tmp/.dify/{env_id}``.
|
||||
|
||||
ROOT: Final[str] = "/tmp/.dify"
|
||||
PATH: Final[str] = "/tmp/.dify/bin/dify"
|
||||
PATH_PATTERN: Final[str] = "dify-cli-{os}-{arch}"
|
||||
Every sandbox environment gets its own directory tree so that
|
||||
concurrent sessions on the same host (e.g. SSH provider) never
|
||||
collide on config files or CLI binaries.
|
||||
|
||||
Class-level constants (``CONFIG_FILENAME``, ``PATH_PATTERN``) are
|
||||
safe to share; all path attributes are instance-level and derived
|
||||
from the ``env_id`` passed at construction time.
|
||||
"""
|
||||
|
||||
# --- class-level constants (no path component) ---
|
||||
CONFIG_FILENAME: Final[str] = ".dify_cli.json"
|
||||
TOOLS_ROOT: Final[str] = "/tmp/.dify/tools"
|
||||
GLOBAL_TOOLS_PATH: Final[str] = "/tmp/.dify/tools/global"
|
||||
PATH_PATTERN: Final[str] = "dify-cli-{os}-{arch}"
|
||||
|
||||
# --- instance attributes ---
|
||||
root: str
|
||||
bin_path: str
|
||||
tools_root: str
|
||||
global_tools_path: str
|
||||
|
||||
def __init__(self, env_id: str) -> None:
|
||||
self.root = f"/tmp/.dify/{env_id}"
|
||||
self.bin_path = f"{self.root}/bin/dify"
|
||||
self.tools_root = f"{self.root}/tools"
|
||||
self.global_tools_path = f"{self.root}/tools/global"
|
||||
|
||||
|
||||
class AppAssets:
|
||||
"""App Assets constants (relative path - stays in sandbox workdir)"""
|
||||
"""App Assets constants.
|
||||
|
||||
``PATH`` is a relative path resolved by each provider against its
|
||||
own workspace root — already isolated. ``zip_path`` is an absolute
|
||||
temp path and must be namespaced per environment to avoid collisions.
|
||||
"""
|
||||
|
||||
PATH: Final[str] = "skills"
|
||||
ZIP_PATH: Final[str] = "/tmp/assets.zip"
|
||||
|
||||
zip_path: str
|
||||
|
||||
def __init__(self, env_id: str) -> None:
|
||||
self.zip_path = f"/tmp/.dify/{env_id}/assets.zip"
|
||||
|
||||
@ -18,6 +18,7 @@ class AppAssetsInitializer(AsyncSandboxInitializer):
|
||||
|
||||
# Load published app assets and unzip the artifact bundle.
|
||||
vm = sandbox.vm
|
||||
assets = AppAssets(sandbox.id)
|
||||
asset_storage = AppAssetService.get_storage()
|
||||
key = AssetPaths.build_zip(ctx.tenant_id, ctx.app_id, ctx.assets_id)
|
||||
download_url = asset_storage.get_download_url(key)
|
||||
@ -25,7 +26,7 @@ class AppAssetsInitializer(AsyncSandboxInitializer):
|
||||
(
|
||||
pipeline(vm)
|
||||
.add(
|
||||
["curl", "-fsSL", download_url, "-o", AppAssets.ZIP_PATH],
|
||||
["curl", "-fsSL", download_url, "-o", assets.zip_path],
|
||||
error_message="Failed to download assets zip",
|
||||
)
|
||||
# Create the assets directory first to ensure it exists even if zip is empty
|
||||
@ -36,7 +37,7 @@ class AppAssetsInitializer(AsyncSandboxInitializer):
|
||||
# unzip with silent error and return 1 if the zip is empty
|
||||
# FIXME(Mairuis): should use a more robust way to check if the zip is empty
|
||||
.add(
|
||||
["sh", "-c", f"unzip {AppAssets.ZIP_PATH} -d {AppAssets.PATH} 2>/dev/null || [ $? -eq 1 ]"],
|
||||
["sh", "-c", f"unzip {assets.zip_path} -d {AppAssets.PATH} 2>/dev/null || [ $? -eq 1 ]"],
|
||||
error_message="Failed to unzip assets",
|
||||
)
|
||||
# Ensure directories have execute permission for traversal and files are readable
|
||||
|
||||
@ -28,27 +28,29 @@ class DifyCliInitializer(AsyncSandboxInitializer):
|
||||
|
||||
def initialize(self, sandbox: Sandbox, ctx: SandboxInitializeContext) -> None:
|
||||
vm = sandbox.vm
|
||||
cli = DifyCli(sandbox.id)
|
||||
|
||||
# FIXME(Mairuis): should be more robust, effectively.
|
||||
binary = self._locator.resolve(vm.metadata.os, vm.metadata.arch)
|
||||
|
||||
pipeline(vm).add(
|
||||
["mkdir", "-p", f"{DifyCli.ROOT}/bin"], error_message="Failed to create dify CLI directory"
|
||||
["mkdir", "-p", f"{cli.root}/bin"], error_message="Failed to create dify CLI directory"
|
||||
).execute(raise_on_error=True)
|
||||
|
||||
vm.upload_file(DifyCli.PATH, BytesIO(binary.path.read_bytes()))
|
||||
vm.upload_file(cli.bin_path, BytesIO(binary.path.read_bytes()))
|
||||
|
||||
pipeline(vm).add(
|
||||
[
|
||||
"sh",
|
||||
"-c",
|
||||
f"cat '{DifyCli.PATH}' > '{DifyCli.PATH}.tmp' && "
|
||||
f"mv '{DifyCli.PATH}.tmp' '{DifyCli.PATH}' && "
|
||||
f"chmod +x '{DifyCli.PATH}'",
|
||||
f"cat '{cli.bin_path}' > '{cli.bin_path}.tmp' && "
|
||||
f"mv '{cli.bin_path}.tmp' '{cli.bin_path}' && "
|
||||
f"chmod +x '{cli.bin_path}'",
|
||||
],
|
||||
error_message="Failed to mark dify CLI as executable",
|
||||
).execute(raise_on_error=True)
|
||||
|
||||
logger.info("Dify CLI uploaded to sandbox, path=%s", DifyCli.PATH)
|
||||
logger.info("Dify CLI uploaded to sandbox, path=%s", cli.bin_path)
|
||||
|
||||
bundle = sandbox.attrs.get(SkillAttrs.BUNDLE)
|
||||
if bundle is None or bundle.get_tool_dependencies().is_empty():
|
||||
@ -62,16 +64,16 @@ class DifyCliInitializer(AsyncSandboxInitializer):
|
||||
)
|
||||
|
||||
pipeline(vm).add(
|
||||
["mkdir", "-p", DifyCli.GLOBAL_TOOLS_PATH], error_message="Failed to create global tools dir"
|
||||
["mkdir", "-p", cli.global_tools_path], error_message="Failed to create global tools dir"
|
||||
).execute(raise_on_error=True)
|
||||
|
||||
config = DifyCliConfig.create(self._cli_api_session, ctx.tenant_id, bundle.get_tool_dependencies())
|
||||
config_json = json.dumps(config.model_dump(mode="json"), ensure_ascii=False)
|
||||
config_path = f"{DifyCli.GLOBAL_TOOLS_PATH}/{DifyCli.CONFIG_FILENAME}"
|
||||
config_path = f"{cli.global_tools_path}/{DifyCli.CONFIG_FILENAME}"
|
||||
vm.upload_file(config_path, BytesIO(config_json.encode("utf-8")))
|
||||
|
||||
pipeline(vm, cwd=DifyCli.GLOBAL_TOOLS_PATH).add(
|
||||
[DifyCli.PATH, "init"], error_message="Failed to initialize Dify CLI"
|
||||
pipeline(vm, cwd=cli.global_tools_path).add(
|
||||
[cli.bin_path, "init"], error_message="Failed to initialize Dify CLI"
|
||||
).execute(raise_on_error=True)
|
||||
|
||||
logger.info("Global tools initialized, path=%s, tool_count=%d", DifyCli.GLOBAL_TOOLS_PATH, len(self._tools))
|
||||
logger.info("Global tools initialized, path=%s, tool_count=%d", cli.global_tools_path, len(self._tools))
|
||||
|
||||
@ -46,9 +46,9 @@ class SandboxManager:
|
||||
cls._shards[shard_index] = new_shard
|
||||
|
||||
logger.debug(
|
||||
"Registered sandbox: sandbox_id=%s, vm_id=%s, app_id=%s",
|
||||
"Registered sandbox: sandbox_id=%s, id=%s, app_id=%s",
|
||||
sandbox_id,
|
||||
sandbox.vm.metadata.id,
|
||||
sandbox.id,
|
||||
sandbox.app_id,
|
||||
)
|
||||
|
||||
@ -71,9 +71,9 @@ class SandboxManager:
|
||||
cls._shards[shard_index] = new_shard
|
||||
|
||||
logger.debug(
|
||||
"Unregistered sandbox: sandbox_id=%s, vm_id=%s",
|
||||
"Unregistered sandbox: sandbox_id=%s, id=%s",
|
||||
sandbox_id,
|
||||
sandbox.vm.metadata.id,
|
||||
sandbox.id,
|
||||
)
|
||||
return sandbox
|
||||
|
||||
|
||||
@ -3,6 +3,7 @@ from __future__ import annotations
|
||||
import logging
|
||||
import threading
|
||||
from typing import TYPE_CHECKING
|
||||
from uuid import uuid4
|
||||
|
||||
from libs.attr_map import AttrMap
|
||||
|
||||
@ -14,6 +15,18 @@ logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
class Sandbox:
|
||||
"""Represents a single sandbox environment.
|
||||
|
||||
Each ``Sandbox`` owns a stable, path-safe ``id`` (a 32-char hex
|
||||
UUID4) that is independent of the underlying provider's environment
|
||||
ID. Use ``sandbox.id`` for any path or resource namespacing
|
||||
(e.g. ``DifyCli(sandbox.id)``).
|
||||
|
||||
The raw provider identifier is still accessible via
|
||||
``sandbox.vm.metadata.id`` when needed (logging, API calls back to
|
||||
the provider, etc.).
|
||||
"""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
*,
|
||||
@ -24,6 +37,7 @@ class Sandbox:
|
||||
app_id: str,
|
||||
assets_id: str,
|
||||
) -> None:
|
||||
self._id = uuid4().hex
|
||||
self._vm = vm
|
||||
self._storage = storage
|
||||
self._tenant_id = tenant_id
|
||||
@ -35,6 +49,11 @@ class Sandbox:
|
||||
self._cancel_event = threading.Event()
|
||||
self._init_error: Exception | None = None
|
||||
|
||||
@property
|
||||
def id(self) -> str:
|
||||
"""Stable, path-safe identifier for this sandbox (UUID4 hex)."""
|
||||
return self._id
|
||||
|
||||
@property
|
||||
def attrs(self) -> AttrMap:
|
||||
return self._attributes
|
||||
@ -100,7 +119,7 @@ class Sandbox:
|
||||
|
||||
def release(self) -> None:
|
||||
self.cancel_init()
|
||||
sandbox_id = self._vm.metadata.id
|
||||
sandbox_id = self.id
|
||||
try:
|
||||
self._storage.unmount(self._vm)
|
||||
logger.info("Sandbox storage unmounted: sandbox_id=%s", sandbox_id)
|
||||
|
||||
Reference in New Issue
Block a user