mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 21:26:39 +00:00
fix(channels): grant sandbox read bits to Feishu/DingTalk inbound uploads (#5998)
The channel manager persists inbound attachments with `write_upload_file_no_symlink` (root-owned, mode 0o600) and then relaxes them to group/other readable via `_make_inbound_file_sandbox_readable`. In AIO/Docker sandbox mode the sandbox runs as a non-root user on the bind-mounted thread directory, so without those bits the file is unreadable and the agent sees an empty attachment. The HTTP upload route applies the same permit. `FeishuChannel._receive_single_file` and `DingTalkChannel._receive_single_file` persist their own bytes and hand back a virtual path instead of routing content through the manager, so neither reached that permit: - Feishu leaves `msg.files` populated with `file_key` descriptors, but the manager's `_read_http_inbound_file` requires a `url` key, so its ingest pass reads no data and skips the upload. - DingTalk clears `msg.files`, so the ingest pass returns immediately. Telegram is unaffected because it hands raw bytes to the manager through `INBOUND_FILE_CONTENT_KEY`, which is why it was covered when the permit was added. Apply `apply_upload_sandbox_permits(..., S_IRGRP | S_IROTH)` right after each channel's own write, off the event loop, mirroring the manager and HTTP paths. Tests: `test_channel_file_attachments.py` gains a mounted-provider regression per channel asserting the persisted upload is group/other readable.
This commit is contained in:
parent
712a11c0cd
commit
8712d495e2
@ -6,6 +6,7 @@ import asyncio
|
||||
import json
|
||||
import logging
|
||||
import re
|
||||
import stat
|
||||
import threading
|
||||
import time
|
||||
from pathlib import Path
|
||||
@ -21,7 +22,13 @@ from app.channels.sandbox_files import sync_file_to_thread_sandbox
|
||||
from deerflow.config.paths import VIRTUAL_PATH_PREFIX, get_paths
|
||||
from deerflow.runtime.user_context import get_effective_user_id
|
||||
from deerflow.sandbox.sandbox_provider import get_sandbox_provider
|
||||
from deerflow.uploads.manager import UnsafeUploadPathError, claim_unique_filename, normalize_filename, write_upload_file_no_symlink
|
||||
from deerflow.uploads.manager import (
|
||||
UnsafeUploadPathError,
|
||||
apply_upload_sandbox_permits,
|
||||
claim_unique_filename,
|
||||
normalize_filename,
|
||||
write_upload_file_no_symlink,
|
||||
)
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@ -678,6 +685,10 @@ class DingTalkChannel(Channel):
|
||||
|
||||
try:
|
||||
resolved_target = await asyncio.to_thread(_persist)
|
||||
# Root-written uploads are 0o600, which the non-root sandbox cannot
|
||||
# read on a bind-mounted thread dir; grant group/other read like the
|
||||
# channel manager's inbound-file path and the HTTP upload route.
|
||||
await asyncio.to_thread(apply_upload_sandbox_permits, resolved_target, stat.S_IRGRP | stat.S_IROTH)
|
||||
except (OSError, UnsafeUploadPathError):
|
||||
logger.exception("[DingTalk] failed to persist downloaded file: %s", safe_filename)
|
||||
return ""
|
||||
|
||||
@ -6,6 +6,7 @@ import asyncio
|
||||
import json
|
||||
import logging
|
||||
import re
|
||||
import stat
|
||||
import threading
|
||||
import time
|
||||
from typing import Any, Literal
|
||||
@ -27,7 +28,12 @@ from app.channels.sandbox_files import sync_file_to_thread_sandbox
|
||||
from deerflow.config.paths import VIRTUAL_PATH_PREFIX, get_paths
|
||||
from deerflow.runtime.user_context import get_effective_user_id
|
||||
from deerflow.sandbox.sandbox_provider import get_sandbox_provider
|
||||
from deerflow.uploads.manager import claim_unique_filename, normalize_filename, write_upload_file_no_symlink
|
||||
from deerflow.uploads.manager import (
|
||||
apply_upload_sandbox_permits,
|
||||
claim_unique_filename,
|
||||
normalize_filename,
|
||||
write_upload_file_no_symlink,
|
||||
)
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
PENDING_CLARIFICATION_TTL_SECONDS = 30 * 60
|
||||
@ -488,6 +494,10 @@ class FeishuChannel(Channel):
|
||||
|
||||
try:
|
||||
resolved_target = await asyncio.to_thread(_persist)
|
||||
# Root-written uploads are 0o600, which the non-root sandbox cannot
|
||||
# read on a bind-mounted thread dir; grant group/other read like the
|
||||
# channel manager's inbound-file path and the HTTP upload route.
|
||||
await asyncio.to_thread(apply_upload_sandbox_permits, resolved_target, stat.S_IRGRP | stat.S_IROTH)
|
||||
except (OSError, ValueError, RuntimeError):
|
||||
logger.exception("[Feishu] failed to persist downloaded resource: %s, type=%s", safe_filename, type)
|
||||
return f"Failed to obtain the [{type}]"
|
||||
|
||||
@ -569,6 +569,97 @@ class TestInboundFileSandboxPerms:
|
||||
assert mode & stat.S_IRGRP
|
||||
assert mode & stat.S_IROTH
|
||||
|
||||
def test_feishu_receive_file_makes_file_sandbox_readable(self, tmp_path, monkeypatch):
|
||||
from io import BytesIO
|
||||
|
||||
from app.channels.feishu import FeishuChannel
|
||||
from deerflow.config.paths import Paths
|
||||
|
||||
monkeypatch.setattr("app.channels.feishu.get_paths", lambda: Paths(str(tmp_path)))
|
||||
monkeypatch.setattr("app.channels.feishu.get_sandbox_provider", _MountedProvider)
|
||||
|
||||
channel = FeishuChannel(MessageBus(), {"app_id": "test", "app_secret": "test"})
|
||||
channel._GetMessageResourceRequest = MagicMock()
|
||||
builder = MagicMock()
|
||||
builder.message_id.return_value = builder
|
||||
builder.file_key.return_value = builder
|
||||
builder.type.return_value = builder
|
||||
builder.build.return_value = object()
|
||||
channel._GetMessageResourceRequest.builder.return_value = builder
|
||||
response = MagicMock()
|
||||
response.success.return_value = True
|
||||
response.file = BytesIO(b"DATA")
|
||||
response.file_name = "report.pdf"
|
||||
channel._api_client = MagicMock()
|
||||
channel._api_client.im.v1.message_resource.get.return_value = response
|
||||
|
||||
msg = InboundMessage(
|
||||
channel_name="feishu",
|
||||
chat_id="chat-1",
|
||||
user_id="ou-user",
|
||||
thread_ts="message-1",
|
||||
text="[file]",
|
||||
files=[{"file_key": "file-key"}],
|
||||
)
|
||||
|
||||
_run(channel.receive_file(msg, "thread-1", user_id="ou-user"))
|
||||
|
||||
dest = tmp_path / "users" / "ou-user" / "threads" / "thread-1" / "user-data" / "uploads" / "report.pdf"
|
||||
assert dest.read_bytes() == b"DATA"
|
||||
mode = stat.S_IMODE(os.stat(dest).st_mode)
|
||||
# Feishu persists the attachment itself (the manager's URL-based
|
||||
# _ingest_inbound_files pass cannot read a file_key descriptor), so it
|
||||
# owes the sandbox the same group/other read bits the manager grants.
|
||||
assert mode & stat.S_IRGRP
|
||||
assert mode & stat.S_IROTH
|
||||
|
||||
def test_dingtalk_receive_file_makes_file_sandbox_readable(self, tmp_path, monkeypatch):
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
from app.channels.dingtalk import DingTalkChannel
|
||||
from deerflow.config.paths import Paths
|
||||
|
||||
monkeypatch.setattr("app.channels.dingtalk.get_paths", lambda: Paths(str(tmp_path)))
|
||||
monkeypatch.setattr("app.channels.dingtalk.get_sandbox_provider", _MountedProvider)
|
||||
|
||||
channel = DingTalkChannel(MessageBus(), config={})
|
||||
channel._download_by_code = AsyncMock(return_value=b"DATA")
|
||||
|
||||
msg = InboundMessage(
|
||||
channel_name="dingtalk",
|
||||
chat_id="chat-1",
|
||||
user_id="user-1",
|
||||
text="[file]",
|
||||
files=[{"type": "file", "download_code": "code-1", "filename": "report.pdf"}],
|
||||
)
|
||||
|
||||
_run(channel.receive_file(msg, "thread-1", user_id="user-1"))
|
||||
|
||||
dest = tmp_path / "users" / "user-1" / "threads" / "thread-1" / "user-data" / "uploads" / "report.pdf"
|
||||
assert dest.read_bytes() == b"DATA"
|
||||
mode = stat.S_IMODE(os.stat(dest).st_mode)
|
||||
# DingTalk clears msg.files, so the manager's ingest pass never runs for
|
||||
# it at all; the permit has to be applied here or nowhere.
|
||||
assert mode & stat.S_IRGRP
|
||||
assert mode & stat.S_IROTH
|
||||
|
||||
|
||||
class _MountedProvider:
|
||||
"""A `uses_thread_data_mounts` provider: the sandbox reads the persisted
|
||||
upload over the bind mount, so no bytes are copied and the file's own mode
|
||||
is the only thing standing between the sandbox and a readable attachment."""
|
||||
|
||||
uses_thread_data_mounts = True
|
||||
|
||||
def acquire(self, thread_id=None, *, user_id=None):
|
||||
raise AssertionError("mounted uploads must not acquire a sandbox")
|
||||
|
||||
async def acquire_async(self, thread_id=None, *, user_id=None):
|
||||
raise AssertionError("mounted uploads must not acquire a sandbox")
|
||||
|
||||
def get(self, sandbox_id):
|
||||
raise AssertionError("mounted uploads must not look up a sandbox")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Channel base class _on_outbound with attachments
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user