diff --git a/README.md b/README.md index 1720554cb..18b1b70b4 100644 --- a/README.md +++ b/README.md @@ -961,9 +961,9 @@ DeerFlow doesn't just *talk* about doing things. It has its own computer. Each task gets its own execution environment with a full filesystem view — skills, workspace, uploads, outputs. The agent reads, writes, and edits files. It can view images and, when configured safely, execute shell commands. -Uploads from the Web UI, embedded client, and IM channels share one collision-safe storage rule. A completed payload is published only if its candidate name does not exist; concurrent `report.pdf` uploads become `report.pdf`, `report_1.pdf`, `report_2.pdf`, and so on without replacing one another. Collision suffixes remain within the filesystem's 255-byte UTF-8 component limit, including names whose original suffix consumes nearly the whole limit. A busy candidate lease is treated as another collision instead of waiting, so inverse multi-file batches cannot deadlock. The selected name is leased through conversion and sandbox synchronization, so deleting that exact name waits for its active upload lifecycle while unrelated filenames continue concurrently. Internal staging names matching `.upload-*.part` and basenames that cannot be represented losslessly on Windows are rejected for new uploads; exact legacy POSIX names already returned by the list endpoint remain deletable after upgrade. +Uploads from the Web UI, embedded client, and IM channels share one collision-safe storage rule. A completed payload is published only if its candidate name does not exist; concurrent `report.pdf` uploads become `report.pdf`, `report_1.pdf`, `report_2.pdf`, and so on without replacing one another. Collision suffixes remain within the filesystem's 255-byte UTF-8 component limit, including names whose original suffix consumes nearly the whole limit. A busy candidate lease is treated as another collision instead of waiting, so inverse multi-file batches cannot deadlock. The selected name is leased through conversion and sandbox synchronization, so deleting that exact name waits for its active upload lifecycle while unrelated filenames continue concurrently. Internal staging names matching `.upload-*.part` and basenames that cannot be represented losslessly on Windows are rejected for new uploads and reported through `skipped_files`; exact legacy POSIX names already returned by the list endpoint—including literal backslashes and names made only from dots/spaces—remain deletable after upgrade. -Optional document conversions are system-owned assets under `/mnt/user-data/.upload-conversions/`. Normal targets use `.md`; names that would exceed the filesystem component limit use a deterministic UTF-8-safe prefix plus the full SHA-256 digest. The exact generated path is returned through the upload response and omitted from the primary upload listing. Mounted AIO sandboxes expose this namespace through a read-only mount, and Local structured file APIs enforce the same rule through path mappings. Local host bash is outside that mapping boundary and must remain disabled for untrusted tasks. Non-mounted remote providers receive a private synchronized copy that may be writable but cannot mutate the authoritative host conversion or lock state. Deleting a primary removes only its exact generated asset and never infers that a user-uploaded sibling such as `uploads/report.md` is disposable. +Optional document conversions are system-owned assets under `/mnt/user-data/.upload-conversions/`. Normal targets use `.md`; names that would exceed the filesystem component limit use a deterministic UTF-8-safe prefix plus the full SHA-256 digest. The exact generated path is returned through the upload response and omitted from the primary upload listing. Mounted AIO sandboxes expose this namespace through a read-only mount; the remote Provisioner verifies its exact user/thread source, and a mount-contract version in deterministic sandbox IDs prevents reuse of pre-upgrade containers that lack it. Local structured file APIs enforce the same rule through path mappings. Local host bash is outside that mapping boundary and must remain disabled for untrusted tasks. Non-mounted remote providers receive a private synchronized copy that may be writable but cannot mutate the authoritative host conversion or lock state. Direct Markdown uploads supply their own outline/preview, while other formats use only their exact generated asset. Deleting a primary removes only its exact generated asset and never infers that a user-uploaded sibling such as `uploads/report.md` is disposable. The built-in `grep` tool searches either one text file or all matching text files below a directory, so an agent can search an uploaded document directly without first broadening the request to the entire uploads directory. diff --git a/backend/AGENTS.md b/backend/AGENTS.md index 02aff7fbb..025bd2597 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -1461,12 +1461,12 @@ Multi-file upload with automatic document conversion: - Reuses one conversion worker per request when called from an active event loop - Files stored in thread-isolated directories under the resolving user's bucket (`users/{user_id}/threads/{thread_id}/user-data/uploads`). For IM channels the owner is threaded explicitly via the `user_id=` kwarg (see IM Channels → Owner-scoped file storage); HTTP/embedded callers resolve it from `get_effective_user_id()` - Every ingress stages a complete payload and atomically publishes it without replacing an existing entry. Collisions across requests, processes, HTTP, embedded, and IM adapters use `name.ext`, `name_1.ext`, `name_2.ext`; storage that cannot provide atomic no-replace publication fails explicitly. -- Exact-name generation leases use a portable NFC-plus-casefold coordination key with Win32 trailing-dot/space folding, so filesystem aliases cannot bypass an active generation; filenames that Windows cannot represent losslessly are rejected before staging, while exact legacy POSIX basenames remain deletable after upgrade. The original filename remains the published name. Publication tries each candidate lease without blocking and treats a busy canonical key as a collision, so same-batch and inverse concurrent batches advance to a UTF-8-bounded `_N` candidate instead of deadlocking while retaining earlier generations; pathological long suffixes fall back to truncating the complete basename. Alias-based deletion resolves the primary's actual directory entry by inode, requires that entry to remain exclusive, and only then derives a long-name conversion path. Final lease release is the commit point: cancellation newly arriving during release is delayed and swallowed so a committed upload is returned as success rather than an indeterminate cancelled result. Embedded-client finalization attempts every lease release and conversion-pool shutdown independently, logging cleanup failures rather than changing an already-committed response or stranding later leases. +- Exact-name generation leases use a portable NFC-plus-casefold coordination key with Win32 trailing-dot/space folding, so filesystem aliases cannot bypass an active generation; filenames that Windows cannot represent losslessly are rejected before staging, while exact legacy POSIX basenames remain deletable after upgrade. Legacy deletion has a POSIX-only lease mode for literal backslashes and components made entirely from dots/spaces; it does not weaken new-upload validation. The original filename remains the published name. Publication tries each candidate lease without blocking and treats a busy canonical key as a collision, so same-batch and inverse concurrent batches advance to a UTF-8-bounded `_N` candidate instead of deadlocking while retaining earlier generations; pathological long suffixes fall back to truncating the complete basename. Alias-based deletion resolves the primary's actual directory entry by inode, requires that entry to remain exclusive, and only then derives a long-name conversion path. Final lease release is the commit point: cancellation newly arriving during release is delayed and swallowed so a committed upload is returned as success rather than an indeterminate cancelled result. Embedded-client finalization attempts every lease release and conversion-pool shutdown independently, logging cleanup failures rather than changing an already-committed response or stranding later leases. - Filenames containing NUL, `<`, `>`, or reserved model-context boundary markers are rejected before staging so accepted filenames and exact virtual paths remain lossless in model-visible upload context. Legacy files discovered on disk are still neutralized when listed. - Gateway HTTP uploads use same-directory `.upload-*.part` staging files. Each active stage holds a cross-process liveness lock under `.upload-conversions/.locks/stages/`; startup cleanup skips held stages and sweeps only crash-orphaned files. Cancellation during staging creation drains the worker and aborts the returned stage before propagating. Staging files are hidden from upload listings, agent upload context, and sandbox listing/search tools. -- Generated Markdown is owned by `user-data/.upload-conversions/.md` and is omitted from primary upload listings. Deletion removes only the selected primary and that exact generated asset; it never guesses or deletes a legacy/user-owned `uploads/.md` sibling. +- Generated Markdown is owned by `user-data/.upload-conversions/.md` and is omitted from primary upload listings. Deletion removes only the selected primary and that exact generated asset; it never guesses or deletes a legacy/user-owned `uploads/.md` sibling. Outline extraction reads this owned conversion for non-Markdown uploads and reads a verified regular primary directly when the uploaded file itself is Markdown. - Gateway HTTP upload/list/delete handlers offload filesystem work through `deerflow.utils.file_io.run_file_io`, a dedicated ContextVar-preserving file IO executor; only operations that may block waiting for a name lease use the separate lease-wait pool. Work needed by an existing lease holder and non-blocking publication stays on the general pool, so waiters cannot starve conversion, rollback, or release. Cold sandbox-provider construction is also offloaded. Gateway, embedded-client, and IM ingresses share provider-aware publication: mounted providers make the exact host paths sandbox-readable; non-mounted providers acquire the sandbox and synchronize the primary plus generated conversion to their exact virtual paths. Each ingress records attempted remote paths before the write can commit and, on later failure or cancellation, calls the provider-neutral `Sandbox.remove_file()` for those paths before host rollback and lease release; the command fallback requires a per-call unpredictable exact success trailer. Embedded multi-file calls retain every publication and receipt until the whole response is built, then roll back the complete batch on failure. WeChat download publication uses the cancellation-safe async lease adapter, so cancellation drains and rolls back a publication worker that completes late. -- Mounted upload paths skip both sandbox acquisition and per-file synchronization. For AIO remote/provisioner deployments this requires an explicit, accurate `sandbox.thread_data_mounts: true`; omission preserves backend auto-detection. +- Mounted upload paths skip both sandbox acquisition and per-file synchronization. For AIO remote/provisioner deployments this requires an explicit, accurate `sandbox.thread_data_mounts: true`; omission preserves backend auto-detection. The remote mount allowlist forwards `/mnt/user-data/.upload-conversions` only when it is the read-only sibling of the same thread's uploads directory; the Provisioner independently verifies the exact user/thread host source and creates a read-only nested hostPath/PVC mount over the writable `/mnt/user-data` parent. `SANDBOX_MOUNT_CONTRACT_VERSION` is included in deterministic AIO sandbox IDs, so a rolling upgrade that adds a required mount cold-starts the new identity instead of reusing an older container that lacks it; old instances remain under normal orphan cleanup. - Agent receives uploaded file list via `UploadsMiddleware` See [docs/FILE_UPLOAD.md](docs/FILE_UPLOAD.md) for details. diff --git a/backend/app/gateway/routers/uploads.py b/backend/app/gateway/routers/uploads.py index bdacee3cb..c3ffe596f 100644 --- a/backend/app/gateway/routers/uploads.py +++ b/backend/app/gateway/routers/uploads.py @@ -423,6 +423,7 @@ async def upload_files( original_filename = normalize_filename(file.filename) except ValueError: logger.warning(f"Skipping file with unsafe filename: {file.filename!r}") + skipped_files.append(file.filename) continue publication, file_size, total_size = await _write_upload_file_with_limits( diff --git a/backend/docs/API.md b/backend/docs/API.md index 706ef9b4b..e98a829a5 100644 --- a/backend/docs/API.md +++ b/backend/docs/API.md @@ -636,9 +636,9 @@ Content-Type: multipart/form-data - Excel (`.xls`, `.xlsx`) - Word (`.doc`, `.docx`) -All upload entry points publish complete payloads without replacing an existing name. Concurrent collisions are returned as `document.pdf`, `document_1.pdf`, `document_2.pdf`, and so on. Collision candidates remain within the 255-byte UTF-8 component limit; when a pathological suffix consumes nearly the entire component, DeerFlow truncates the complete basename before appending `_N`. A published filename remains leased through conversion, permission adjustment, sandbox synchronization, and response construction; portable case, Unicode-normalization, and Win32 trailing-dot/space aliases share the same coordination key. Publication never waits on a busy candidate lease and advances to `_N`, preventing inverse multi-file batches from deadlocking; deletion waits for the target generation and rejects ambiguous hard-linked identities. Mounted providers make the exact published paths sandbox-readable; non-mounted providers receive exact private copies for Gateway, embedded-client, and IM-channel ingresses. If a non-mounted sandbox update later fails or the request is cancelled, DeerFlow removes every exact remote path attempted by that request before rolling back its host generations. Gateway cancellation also drains and aborts an in-flight staging creation. Final lease release is the commit point: cancellation newly arriving during release is delayed and the already-built successful response is returned. Basenames matching the internal `.upload-*.part` staging pattern, containing NUL, `<`, or `>`, containing reserved model-context boundary markers, or invalid/reserved on Windows are rejected before staging so every accepted model-visible filename and path can be rendered losslessly. Embedded multi-file calls are request-atomic: a later failure rolls back every earlier host and remote generation in that call. +All upload entry points publish complete payloads without replacing an existing name. Concurrent collisions are returned as `document.pdf`, `document_1.pdf`, `document_2.pdf`, and so on. Collision candidates remain within the 255-byte UTF-8 component limit; when a pathological suffix consumes nearly the entire component, DeerFlow truncates the complete basename before appending `_N`. A published filename remains leased through conversion, permission adjustment, sandbox synchronization, and response construction; portable case, Unicode-normalization, and Win32 trailing-dot/space aliases share the same coordination key. Publication never waits on a busy candidate lease and advances to `_N`, preventing inverse multi-file batches from deadlocking; deletion waits for the target generation and rejects ambiguous hard-linked identities. Mounted providers make the exact published paths sandbox-readable; non-mounted providers receive exact private copies for Gateway, embedded-client, and IM-channel ingresses. If a non-mounted sandbox update later fails or the request is cancelled, DeerFlow removes every exact remote path attempted by that request before rolling back its host generations. Gateway cancellation also drains and aborts an in-flight staging creation. Final lease release is the commit point: cancellation newly arriving during release is delayed and the already-built successful response is returned. Basenames matching the internal `.upload-*.part` staging pattern, containing NUL, `<`, or `>`, containing reserved model-context boundary markers, or invalid/reserved on Windows are rejected before staging so every accepted model-visible filename and path can be rendered losslessly; rejected names are returned through `skipped_files` and make the response unsuccessful. Embedded multi-file calls are request-atomic: a later failure rolls back every earlier host and remote generation in that call. -Generated Markdown is stored outside the primary namespace and is not returned by the list endpoint. Normal conversion names are `.md`; if that component would exceed 255 UTF-8 bytes, the response contains a deterministic UTF-8-safe prefix plus the full SHA-256 digest and `.md`. Clients must consume the returned `markdown_*` fields rather than derive the path. Mounted AIO sandboxes use a read-only conversion mount, while Local structured file APIs reject writes through a read-only path mapping; Local host bash is outside that boundary. Non-mounted providers receive a private synchronized copy rather than the authoritative host namespace. Deleting `document.pdf` also deletes only its exact generated conversion; an independent `uploads/document.md` is preserved. +Generated Markdown is stored outside the primary namespace and is not returned by the list endpoint. Normal conversion names are `.md`; if that component would exceed 255 UTF-8 bytes, the response contains a deterministic UTF-8-safe prefix plus the full SHA-256 digest and `.md`. Clients must consume the returned `markdown_*` fields rather than derive the path. Mounted AIO sandboxes use a read-only conversion mount. The remote Provisioner independently verifies its exact user/thread source, and the mount-contract version namespaces deterministic sandbox IDs so pre-upgrade containers without the mount are not reused. Local structured file APIs reject writes through a read-only path mapping; Local host bash is outside that boundary. Non-mounted providers receive a private synchronized copy rather than the authoritative host namespace. Direct Markdown primaries provide their own outline/preview; other formats use only the exact owned conversion. Deleting `document.pdf` also deletes only its exact generated conversion; an independent `uploads/document.md` is preserved. #### List Uploaded Files @@ -681,8 +681,9 @@ DELETE /api/threads/{thread_id}/uploads/{filename} If an upload, conversion, or sandbox synchronization still owns this exact filename, the delete waits for that lifecycle to finish before removing the primary and its generated conversion. Work on unrelated filenames is not serialized. On POSIX deployments, exact -legacy names returned by the list endpoint remain deletable after upgrade even when the -same names would fail the stricter cross-platform validation applied to new uploads. +legacy names returned by the list endpoint—including literal backslashes and components +made only from dots/spaces—remain deletable after upgrade even when the same names would +fail the stricter cross-platform validation applied to new uploads. ### Thread Cleanup diff --git a/backend/docs/FILE_UPLOAD.md b/backend/docs/FILE_UPLOAD.md index 0615e4fe2..a2ea3b2d5 100644 --- a/backend/docs/FILE_UPLOAD.md +++ b/backend/docs/FILE_UPLOAD.md @@ -54,7 +54,7 @@ POST /api/threads/{thread_id}/uploads 所有上传入口都先完整写入同目录暂存文件，再以“不替换已有条目”的原子操作发布。同名碰撞依次命名为 `document.pdf`、`document_1.pdf`、`document_2.pdf`；响应中的 `filename` 和各路径字段始终使用实际发布名。系统内部保留 `.upload-*.part` 作为暂存命名空间；使用该模式的 basename、包含 NUL、`<` 或 `>`、包含保留模型上下文边界标记，或无法在 Windows 上无损表示（如设备名、尾随点/空格或保留字符）的文件名会在创建暂存文件前被拒绝，以保证所有已接受的文件名和 Agent 可见路径都能无损呈现。若请求在 staging 创建尚未返回时被取消，Gateway 会等待创建结束并精确 abort 该临时文件。 -实际发布名会在转换、权限调整、沙箱同步和响应构造期间持有同名租约。大小写、Unicode 规范化及 Win32 尾随后缀等可移植文件系统别名共用同一协调键，避免用别名绕过 generation lease。发布遇到正在使用的协调键时不会等待，而会继续选择 `_N` 候选，因此逆序并发批次不会互相持锁；删除仍会等待目标 generation 生命周期完成，并在 hard-link 歧义下拒绝误报成功。跨进程协调使用 `.upload-conversions/.locks/` 下稳定保留的摘要锁文件，该目录属于内部实现，不应由 Agent 或部署脚本修改或清理。最终 lease release 是明确提交点：如果新的取消恰在 release 期间到达，系统会先完成 release 并返回已构造的成功结果，而不会把已提交文件报告成取消。 +实际发布名会在转换、权限调整、沙箱同步和响应构造期间持有同名租约。大小写、Unicode 规范化及 Win32 尾随后缀等可移植文件系统别名共用同一协调键，避免用别名绕过 generation lease。发布遇到正在使用的协调键时不会等待，而会继续选择 `_N` 候选，因此逆序并发批次不会互相持锁；删除仍会等待目标 generation 生命周期完成，并在 hard-link 歧义下拒绝误报成功。跨进程协调使用 `.upload-conversions/.locks/` 下稳定保留的摘要锁文件，该目录属于内部实现，不应由 Agent 或部署脚本修改或清理。最终 lease release 是明确提交点：如果新的取消恰在 release 期间到达，系统会先完成 release 并返回已构造的成功结果，而不会把已提交文件报告成取消。Gateway 拒绝不安全文件名时会把原名加入 `skipped_files` 并返回 `success: false`，不会把“上传 0 个文件”误报为成功。 ### 2. 查询上传限制 ``` @@ -110,7 +110,7 @@ DELETE /api/threads/{thread_id}/uploads/{filename} } ``` -删除 `document.pdf` 时，会先等待该实际文件名当前正在进行的上传、转换或沙箱同步生命周期结束，然后只额外删除它精确拥有的生成资产。系统不会推断或删除 `uploads/document.md`；该文件可能是用户独立上传的内容。其他文件名不会被这次等待阻塞。在 POSIX 部署上，升级前已经存在且能被列表接口返回的 Windows 非兼容文件名（例如 `CON`、`report?.pdf` 或末尾带空格的名称）仍可按返回的精确名称删除；新上传仍执行严格的跨平台文件名校验。 +删除 `document.pdf` 时，会先等待该实际文件名当前正在进行的上传、转换或沙箱同步生命周期结束，然后只额外删除它精确拥有的生成资产。系统不会推断或删除 `uploads/document.md`；该文件可能是用户独立上传的内容。其他文件名不会被这次等待阻塞。在 POSIX 部署上，升级前已经存在且能被列表接口返回的 Windows 非兼容文件名（例如 `CON`、`report?.pdf`、含反斜杠、仅由点/空格组成或末尾带空格的名称）仍可按返回的精确名称删除；新上传仍执行严格的跨平台文件名校验。 ## 支持的文档格式 @@ -130,7 +130,9 @@ Deletion: 删除 report.pdf 时只删除 .upload-conversions/report.pdf.md； /mnt/user-data/uploads/report.md 永远不会被推断为生成文件或自动删除。 ``` -通常生成名为 `<实际主文件名>.md`。如果这一文件名组件会超过 255 个 UTF-8 字节，系统会使用 UTF-8 安全截断的主文件名前缀、完整 SHA-256 摘要和 `.md`，并在响应中返回精确的 `markdown_*` 路径。上传名称碰撞产生的 `_N` 名称同样始终限制在 255 个 UTF-8 字节内；极端情况下，如果后缀本身占满几乎整个组件，系统会截断完整原名后再追加 `_N`。客户端和 Agent 不应自行拼接生成路径。AIO 挂载模式把 `.upload-conversions` 显式挂载为只读；Local 的结构化文件 API 通过只读路径映射拒绝写入，但可选的 Local 宿主机 bash 不受该映射约束，不应对不受信任任务启用。非挂载远端沙箱得到的是独立同步副本，该副本可能可写，但不会修改宿主机上的权威生成文件或内部锁。 +通常生成名为 `<实际主文件名>.md`。如果这一文件名组件会超过 255 个 UTF-8 字节，系统会使用 UTF-8 安全截断的主文件名前缀、完整 SHA-256 摘要和 `.md`，并在响应中返回精确的 `markdown_*` 路径。上传名称碰撞产生的 `_N` 名称同样始终限制在 255 个 UTF-8 字节内；极端情况下，如果后缀本身占满几乎整个组件，系统会截断完整原名后再追加 `_N`。客户端和 Agent 不应自行拼接生成路径。AIO 挂载模式把 `.upload-conversions` 显式挂载为只读；远端 Provisioner 会再次校验该挂载必须来自同一用户/线程并覆盖在可写父挂载之上。挂载契约版本进入 AIO sandbox ID，因此升级前创建、缺少该目录的容器不会被新版本复用。Local 的结构化文件 API 通过只读路径映射拒绝写入，但可选的 Local 宿主机 bash 不受该映射约束，不应对不受信任任务启用。非挂载远端沙箱得到的是独立同步副本，该副本可能可写，但不会修改宿主机上的权威生成文件或内部锁。 + +直接上传的 `.md` 主文件会从自身提取 outline/preview；PDF、Office 等其他格式只读取其精确拥有的 `.upload-conversions` 转换件，不会把用户独立上传的同名 Markdown 误认为转换结果。 默认情况下，自动转换是关闭的，以避免在网关主机上对不受信任的 Office/PDF 上传执行解析。只有在受信任部署中明确接受此风险时，才应将 `uploads.auto_convert_documents` 设置为 `true`。 diff --git a/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox_provider.py b/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox_provider.py index b29ff8a3e..c3434b502 100644 --- a/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox_provider.py +++ b/backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox_provider.py @@ -61,6 +61,8 @@ from .ownership import ( from .remote_backend import RemoteSandboxBackend from .sandbox_info import SandboxInfo +SANDBOX_MOUNT_CONTRACT_VERSION = 2 + logger = logging.getLogger(__name__) # Default configuration @@ -751,11 +753,12 @@ class AioSandboxProvider(WarmPoolLifecycleMixin[SandboxInfo], SandboxProvider): Includes user_id so a previously-created default-bucket sandbox cannot be reused for an auth/channel run that should mount a user-scoped bucket. - During a mixed-version rollout, older 8-character containers are not - reused under the new 16-character identity. They remain eligible for + The mount-contract version is part of the identity. During a rolling + upgrade, a container created before a required mount was added is never + discovered or reclaimed by the new provider. It remains eligible for normal orphan cleanup while the first new-version acquire cold-starts. """ - return hashlib.sha256(f"{user_id}:{thread_id}".encode()).hexdigest()[:16] + return hashlib.sha256(f"mount-v{SANDBOX_MOUNT_CONTRACT_VERSION}:{user_id}:{thread_id}".encode()).hexdigest()[:16] def _assert_active_identity_available_locked( self, diff --git a/backend/packages/harness/deerflow/community/aio_sandbox/remote_backend.py b/backend/packages/harness/deerflow/community/aio_sandbox/remote_backend.py index f34415907..438f6b8f7 100644 --- a/backend/packages/harness/deerflow/community/aio_sandbox/remote_backend.py +++ b/backend/packages/harness/deerflow/community/aio_sandbox/remote_backend.py @@ -18,6 +18,9 @@ Architecture: from __future__ import annotations import logging +import os +import re +from pathlib import Path, PureWindowsPath import requests @@ -30,6 +33,7 @@ from .sandbox_info import SandboxInfo logger = logging.getLogger(__name__) _PROVISIONER_EXTRA_MOUNT_PATHS = { + "/mnt/user-data/.upload-conversions", "/mnt/acp-workspace", "/mnt/skills/custom", "/mnt/skills/integrations", @@ -38,6 +42,9 @@ _PROVISIONER_EXTRA_MOUNT_PATHS = { "/mnt/integrations/lark-cli/runtime", } +_UPLOADS_CONTAINER_PATH = "/mnt/user-data/uploads" +_UPLOAD_CONVERSIONS_CONTAINER_PATH = "/mnt/user-data/.upload-conversions" + _LARK_CLI_RUNTIME_CONTAINER_PATH = "/mnt/integrations/lark-cli/runtime" _LARK_CLI_CONFIG_CONTAINER_PATH = "/mnt/integrations/lark-cli/config" _LARK_CLI_DATA_CONTAINER_PATH = "/mnt/integrations/lark-cli/data" @@ -69,10 +76,35 @@ def _provisioner_extra_mounts_payload( drop_runtime = provision_lark_cli_runtime or provision_lark_cli_broker + uploads_host_path = next( + (host_path for host_path, container_path, _read_only in extra_mounts if container_path == _UPLOADS_CONTAINER_PATH), + None, + ) + + def expected_conversion_host_path() -> str | None: + if uploads_host_path is None: + return None + if re.match(r"^[A-Za-z]:[\\/]", uploads_host_path) or uploads_host_path.startswith("\\\\") or "\\" in uploads_host_path: + return str(PureWindowsPath(uploads_host_path).parent / ".upload-conversions") + return str(Path(uploads_host_path).parent / ".upload-conversions") + + expected_conversion_path = expected_conversion_host_path() + payload: list[dict[str, object]] = [] for host_path, container_path, read_only in extra_mounts: if container_path not in _PROVISIONER_EXTRA_MOUNT_PATHS: continue + if container_path == _UPLOAD_CONVERSIONS_CONTAINER_PATH: + if not read_only: + raise ValueError("Upload conversion mount must be read-only") + if expected_conversion_path is None: + raise ValueError("Upload conversion mount requires the matching uploads mount") + if re.match(r"^[A-Za-z]:[\\/]", expected_conversion_path) or expected_conversion_path.startswith("\\\\"): + matches_expected = PureWindowsPath(host_path) == PureWindowsPath(expected_conversion_path) + else: + matches_expected = os.path.normpath(host_path) == os.path.normpath(expected_conversion_path) + if not matches_expected: + raise ValueError("Upload conversion mount must belong to the same thread data directory") if drop_runtime and container_path == _LARK_CLI_RUNTIME_CONTAINER_PATH: continue payload.append( diff --git a/backend/packages/harness/deerflow/uploads/lease.py b/backend/packages/harness/deerflow/uploads/lease.py index ca604bf55..df3bad7d4 100644 --- a/backend/packages/harness/deerflow/uploads/lease.py +++ b/backend/packages/harness/deerflow/uploads/lease.py @@ -288,9 +288,19 @@ class UploadNameLease: _state_lock: threading.Lock = field(default_factory=threading.Lock) @classmethod - def acquire(cls, uploads_dir: Path, filename: str) -> "UploadNameLease": + def acquire( + cls, + uploads_dir: Path, + filename: str, + *, + allow_legacy_posix_filename: bool = False, + ) -> "UploadNameLease": """Acquire the stable name lease, blocking until it is available.""" - uploads_dir, coordination_key, digest = cls._validate_request(uploads_dir, filename) + uploads_dir, coordination_key, digest = cls._validate_request( + uploads_dir, + filename, + allow_legacy_posix_filename=allow_legacy_posix_filename, + ) thread_lock_key, thread_lock_entry = _acquire_thread_lock(uploads_dir, coordination_key) lock_file: BinaryIO | None = None try: @@ -312,9 +322,19 @@ class UploadNameLease: raise @classmethod - def try_acquire(cls, uploads_dir: Path, filename: str) -> "UploadNameLease | None": + def try_acquire( + cls, + uploads_dir: Path, + filename: str, + *, + allow_legacy_posix_filename: bool = False, + ) -> "UploadNameLease | None": """Acquire a name lease without waiting, or return ``None`` when busy.""" - uploads_dir, coordination_key, digest = cls._validate_request(uploads_dir, filename) + uploads_dir, coordination_key, digest = cls._validate_request( + uploads_dir, + filename, + allow_legacy_posix_filename=allow_legacy_posix_filename, + ) thread_lock = _try_acquire_thread_lock(uploads_dir, coordination_key) if thread_lock is None: return None @@ -346,15 +366,26 @@ class UploadNameLease: raise @staticmethod - def _validate_request(uploads_dir: Path, filename: str) -> tuple[Path, str, str]: - if not filename or Path(filename).name != filename or "\\" in filename: + def _validate_request( + uploads_dir: Path, + filename: str, + *, + allow_legacy_posix_filename: bool = False, + ) -> tuple[Path, str, str]: + allow_legacy = allow_legacy_posix_filename and os.name != "nt" + if not filename or Path(filename).name != filename or ("\\" in filename and not allow_legacy): raise UnsafeUploadPathError(f"Unsafe upload lease filename: {filename!r}") if len(filename.encode("utf-8")) > 255: raise UnsafeUploadPathError("Upload lease filename is too long") uploads_dir = Path(uploads_dir) coordination_key = portable_name_coordination_key(filename) if not coordination_key: - raise UnsafeUploadPathError(f"Unsafe upload lease filename: {filename!r}") + if not allow_legacy: + raise UnsafeUploadPathError(f"Unsafe upload lease filename: {filename!r}") + # New uploads reject components made entirely from Win32-ignored + # dots/spaces. Existing POSIX entries still need a stable, distinct + # lock key so they can be deleted after upgrade. + coordination_key = f"legacy-posix:{unicodedata.normalize('NFC', filename).casefold()}" digest = hashlib.sha256(coordination_key.encode("utf-8")).hexdigest() return uploads_dir, coordination_key, digest diff --git a/backend/packages/harness/deerflow/uploads/manager.py b/backend/packages/harness/deerflow/uploads/manager.py index 5acb8684c..0303690e3 100644 --- a/backend/packages/harness/deerflow/uploads/manager.py +++ b/backend/packages/harness/deerflow/uploads/manager.py @@ -638,7 +638,11 @@ def delete_file_safe(base_dir: Path, filename: str) -> dict: if isinstance(exc.__cause__, FileNotFoundError): raise FileNotFoundError(f"File not found: {filename}") from exc raise - lease = UploadNameLease.acquire(base_dir, safe_name) + lease = UploadNameLease.acquire( + base_dir, + safe_name, + allow_legacy_posix_filename=True, + ) try: file_path = base_dir / safe_name try: diff --git a/backend/packages/harness/deerflow/utils/file_outline.py b/backend/packages/harness/deerflow/utils/file_outline.py index e1f7524a3..9739f491d 100644 --- a/backend/packages/harness/deerflow/utils/file_outline.py +++ b/backend/packages/harness/deerflow/utils/file_outline.py @@ -7,7 +7,9 @@ the middleware and the ``list_uploaded_files`` tool can use the same code. from __future__ import annotations import logging +import os import re +import stat from pathlib import Path from deerflow.uploads.layout import ( @@ -132,7 +134,8 @@ def extract_outline(md_path: Path) -> list[dict]: def extract_outline_for_file(file_path: Path) -> tuple[list[dict], list[str]]: """Return the document outline and fallback preview for *file_path*. - Looks only for the system-owned Markdown generated for this exact upload. + Uses the primary itself for direct Markdown uploads. Other formats use only + the system-owned Markdown generated for that exact upload. Returns: (outline, preview) where: @@ -142,11 +145,20 @@ def extract_outline_for_file(file_path: Path) -> tuple[list[dict], list[str]]: anchor when outline is empty so the agent has some context. Empty when outline is non-empty (no fallback needed). """ - try: - md_path = existing_conversion_path_for_upload(file_path) - except UnsafeConversionPathError: - logger.warning("Ignoring unsafe generated conversion for %s", file_path.name) - return [], [] + if file_path.suffix.lower() == ".md": + try: + primary_stat = os.lstat(file_path) + except OSError: + return [], [] + if not stat.S_ISREG(primary_stat.st_mode) or primary_stat.st_nlink != 1: + return [], [] + md_path = file_path + else: + try: + md_path = existing_conversion_path_for_upload(file_path) + except UnsafeConversionPathError: + logger.warning("Ignoring unsafe generated conversion for %s", file_path.name) + return [], [] if md_path is None: return [], [] diff --git a/backend/tests/test_aio_sandbox_provider.py b/backend/tests/test_aio_sandbox_provider.py index d85140df0..fb8ebd468 100644 --- a/backend/tests/test_aio_sandbox_provider.py +++ b/backend/tests/test_aio_sandbox_provider.py @@ -275,13 +275,20 @@ def test_get_extra_mounts_provisioner_payload_has_unique_container_paths(tmp_pat payload = remote_backend._provisioner_extra_mounts_payload(mounts) payload_paths = [str(item["container_path"]) for item in payload] assert len(payload_paths) == len(set(payload_paths)) + conversion_payload = next(item for item in payload if item["container_path"] == "/mnt/user-data/.upload-conversions") + assert conversion_payload["read_only"] is True provisioner_module.DEER_FLOW_HOST_BASE_DIR = str(home) - validated = provisioner_module._validated_extra_mounts([provisioner_module.ExtraMount(**item) for item in payload]) + validated = provisioner_module._validated_extra_mounts( + [provisioner_module.ExtraMount(**item) for item in payload], + thread_id="thread-1", + user_id="alice", + ) validated_paths = [mount.container_path for mount in validated] assert len(validated_paths) == len(set(validated_paths)) assert set(validated_paths) == { + "/mnt/user-data/.upload-conversions", "/mnt/acp-workspace", "/mnt/skills/custom", "/mnt/skills/integrations", @@ -291,6 +298,24 @@ def test_get_extra_mounts_provisioner_payload_has_unique_container_paths(tmp_pat } +@pytest.mark.parametrize( + "conversion_mount", + [ + ("/state/users/alice/threads/thread-1/user-data/.upload-conversions", "/mnt/user-data/.upload-conversions", False), + ("/state/users/bob/threads/thread-1/user-data/.upload-conversions", "/mnt/user-data/.upload-conversions", True), + ], +) +def test_provisioner_payload_rejects_unsafe_conversion_mount(conversion_mount): + remote_backend = importlib.import_module("deerflow.community.aio_sandbox.remote_backend") + mounts = [ + ("/state/users/alice/threads/thread-1/user-data/uploads", "/mnt/user-data/uploads", False), + conversion_mount, + ] + + with pytest.raises(ValueError, match="Upload conversion mount"): + remote_backend._provisioner_extra_mounts_payload(mounts) + + def test_join_host_path_preserves_windows_drive_letter_style(): base = r"C:\Users\demo\deer-flow\backend\.deer-flow" @@ -1064,6 +1089,18 @@ def test_aio_wider_id_separates_known_legacy_collision(): ) +def test_aio_mount_contract_upgrade_does_not_reuse_pre_upgrade_container_id(): + aio_mod = importlib.import_module("deerflow.community.aio_sandbox.aio_sandbox_provider") + user_id = "alice" + thread_id = "thread-upgrade" + pre_upgrade_id = hashlib.sha256(f"{user_id}:{thread_id}".encode()).hexdigest()[:16] + + current_id = aio_mod.AioSandboxProvider._deterministic_sandbox_id(thread_id, user_id) + + assert current_id != pre_upgrade_id + assert current_id == hashlib.sha256(f"mount-v{aio_mod.SANDBOX_MOUNT_CONTRACT_VERSION}:{user_id}:{thread_id}".encode()).hexdigest()[:16] + + def test_aio_forced_collision_never_overwrites_active_tenant( tmp_path, monkeypatch, diff --git a/backend/tests/test_client.py b/backend/tests/test_client.py index 3549affbb..f925939f0 100644 --- a/backend/tests/test_client.py +++ b/backend/tests/test_client.py @@ -2625,10 +2625,11 @@ class TestUploads: client.delete_upload("thread-1", "nope.txt") @pytest.mark.skipif(os.name == "nt", reason="POSIX legacy filenames are not representable on Windows") - def test_delete_upload_accepts_legacy_posix_filename(self, client, tmp_path): + @pytest.mark.parametrize("filename", ["report?.pdf", r"report\draft.pdf", "...", " "]) + def test_delete_upload_accepts_legacy_posix_filename(self, client, tmp_path, filename): uploads_dir = tmp_path / "uploads" uploads_dir.mkdir() - legacy = uploads_dir / "report?.pdf" + legacy = uploads_dir / filename legacy.write_bytes(b"legacy") with patch("deerflow.client.get_uploads_dir", return_value=uploads_dir): diff --git a/backend/tests/test_list_uploaded_files_tool.py b/backend/tests/test_list_uploaded_files_tool.py index 703209a2d..b72e9a59d 100644 --- a/backend/tests/test_list_uploaded_files_tool.py +++ b/backend/tests/test_list_uploaded_files_tool.py @@ -11,6 +11,7 @@ from langchain_core.messages import HumanMessage, ToolMessage from deerflow.config.paths import Paths from deerflow.tools.builtins.list_uploaded_files_tool import _format_omitted_summary, _list_uploaded_files_impl, _resolve_thread_id from deerflow.uploads.layout import conversion_path_for_upload, conversion_virtual_path +from deerflow.utils.file_outline import extract_outline_for_file def _paths(tmp_path): @@ -196,6 +197,32 @@ class TestListUploadedFiles: assert result["files"][0]["outline"][0]["title"] == "Heading 1" assert result["files"][0]["outline"][1]["title"] == "Heading 2" + def test_direct_markdown_upload_uses_primary_for_outline(self, tmp_path): + uploads_dir = _uploads_dir(tmp_path) + primary = uploads_dir / "notes.md" + primary.write_text("# Direct heading\n\nBody text.\n", encoding="utf-8") + + result = _list_uploaded_files_impl(include_outline=True, runtime=_runtime(), _paths=_paths(tmp_path)) + + assert result["files"][0]["outline"] == [{"title": "Direct heading", "line": 1}] + + def test_direct_markdown_outline_rejects_symlink_and_hardlink(self, tmp_path): + uploads_dir = _uploads_dir(tmp_path) + outside = tmp_path / "outside.md" + outside.write_text("# Outside heading\n", encoding="utf-8") + symlink = uploads_dir / "symlink.md" + try: + symlink.symlink_to(outside) + except OSError as exc: + if getattr(exc, "winerror", None) == 1314: + pytest.skip("Windows symlink privilege is not available") + raise + hardlink = uploads_dir / "hardlink.md" + os.link(outside, hardlink) + + assert extract_outline_for_file(symlink) == ([], []) + assert extract_outline_for_file(hardlink) == ([], []) + def test_long_filename_result_includes_exact_generated_markdown_path(self, tmp_path): uploads_dir = _uploads_dir(tmp_path) filename = f"{'a' * 250}.pdf" diff --git a/backend/tests/test_provisioner_mount_contract.py b/backend/tests/test_provisioner_mount_contract.py index 403b890e7..6ed0e346f 100644 --- a/backend/tests/test_provisioner_mount_contract.py +++ b/backend/tests/test_provisioner_mount_contract.py @@ -24,5 +24,6 @@ def test_gateway_and_provisioner_extra_mount_contracts_match() -> None: provisioner_paths = _literal_assignment(provisioner_path, "ALLOWED_EXTRA_MOUNT_PATHS") assert gateway_paths == provisioner_paths + assert "/mnt/user-data/.upload-conversions" in gateway_paths assert "/mnt/integrations/lark-cli/runtime" in gateway_paths assert _literal_assignment(provisioner_path, "MAX_EXTRA_MOUNTS") == 9 diff --git a/backend/tests/test_provisioner_pvc_volumes.py b/backend/tests/test_provisioner_pvc_volumes.py index 656654d12..12b2e806b 100644 --- a/backend/tests/test_provisioner_pvc_volumes.py +++ b/backend/tests/test_provisioner_pvc_volumes.py @@ -332,6 +332,52 @@ class TestBuildVolumeMounts: assert extra_mount.name == "extra-0" assert extra_mount.sub_path == "deer-flow/users/alice/integrations/lark-cli/config" + def test_conversion_mount_is_exact_thread_read_only_subpath(self, provisioner_module): + provisioner_module.USERDATA_PVC_NAME = "userdata-pvc" + provisioner_module.DEER_FLOW_HOST_BASE_DIR = "/state" + conversion_mount = provisioner_module.ExtraMount( + host_path="/state/users/alice/threads/thread-1/user-data/.upload-conversions", + container_path="/mnt/user-data/.upload-conversions", + read_only=True, + ) + + mounts = provisioner_module._build_volume_mounts( + "thread-1", + user_id="alice", + extra_mounts=[conversion_mount], + ) + + nested = next(mount for mount in mounts if mount.mount_path == "/mnt/user-data/.upload-conversions") + assert nested.read_only is True + assert nested.sub_path == "deer-flow/users/alice/threads/thread-1/user-data/.upload-conversions" + + @pytest.mark.parametrize( + "mount", + [ + { + "host_path": "/state/users/alice/threads/thread-1/user-data/.upload-conversions", + "container_path": "/mnt/user-data/.upload-conversions", + "read_only": False, + }, + { + "host_path": "/state/users/bob/threads/thread-1/user-data/.upload-conversions", + "container_path": "/mnt/user-data/.upload-conversions", + "read_only": True, + }, + ], + ) + def test_conversion_mount_rejects_writable_or_cross_user_source(self, provisioner_module, mount): + provisioner_module.DEER_FLOW_HOST_BASE_DIR = "/state" + + with pytest.raises(provisioner_module.HTTPException) as exc_info: + provisioner_module._build_volume_mounts( + "thread-1", + user_id="alice", + extra_mounts=[provisioner_module.ExtraMount(**mount)], + ) + + assert exc_info.value.status_code == 400 + def test_extra_mount_rejects_unknown_container_path(self, provisioner_module): """Only first-party managed mount paths are accepted.""" provisioner_module.DEER_FLOW_HOST_BASE_DIR = "/state" diff --git a/backend/tests/test_uploads_manager.py b/backend/tests/test_uploads_manager.py index 7e78d6e80..61ea9f2e7 100644 --- a/backend/tests/test_uploads_manager.py +++ b/backend/tests/test_uploads_manager.py @@ -847,7 +847,7 @@ class TestDeleteFileSafe: delete_file_safe(tmp_path, "nope.txt") @pytest.mark.skipif(os.name == "nt", reason="POSIX legacy filenames are not representable on Windows") - @pytest.mark.parametrize("filename", ["CON", "report?.pdf", "trailing "]) + @pytest.mark.parametrize("filename", ["CON", "report?.pdf", "trailing ", r"report\draft.pdf", "...", " "]) def test_delete_accepts_listed_legacy_posix_filename(self, tmp_path, filename): legacy = tmp_path / filename legacy.write_bytes(b"legacy") diff --git a/backend/tests/test_uploads_router.py b/backend/tests/test_uploads_router.py index 0f4ae71f5..b0bac68ca 100644 --- a/backend/tests/test_uploads_router.py +++ b/backend/tests/test_uploads_router.py @@ -1110,8 +1110,9 @@ def test_upload_files_rejects_dotdot_and_dot_filenames(tmp_path): for bad_name in ["..", "."]: file = UploadFile(filename=bad_name, file=BytesIO(b"data")) result = asyncio.run(call_unwrapped(uploads.upload_files, "thread-local", request=MagicMock(), files=[file], config=SimpleNamespace())) - assert result.success is True + assert result.success is False assert result.files == [], f"Expected no files for unsafe filename {bad_name!r}" + assert result.skipped_files == [bad_name] # Path-traversal prefixes are stripped to the basename and accepted safely file = UploadFile(filename="../etc/passwd", file=BytesIO(b"data")) @@ -1124,6 +1125,31 @@ def test_upload_files_rejects_dotdot_and_dot_filenames(tmp_path): assert [f.name for f in thread_uploads_dir.iterdir()] == ["passwd"] +@pytest.mark.parametrize("bad_name", ["CON", "report?.pdf", ".txt"]) +def test_upload_files_reports_rejected_lossless_filename(tmp_path, bad_name): + thread_uploads_dir = tmp_path / "uploads" + thread_uploads_dir.mkdir(parents=True) + + with ( + patch.object(uploads, "ensure_uploads_dir", return_value=thread_uploads_dir), + patch.object(uploads, "get_sandbox_provider", return_value=_mounted_provider()), + ): + result = asyncio.run( + call_unwrapped( + uploads.upload_files, + "thread-local", + request=MagicMock(), + files=[UploadFile(filename=bad_name, file=BytesIO(b"data"))], + config=SimpleNamespace(), + ) + ) + + assert result.success is False + assert result.files == [] + assert result.skipped_files == [bad_name] + assert result.message == "Successfully uploaded 0 file(s); skipped 1 unsafe file(s)" + + def test_upload_files_renames_around_preexisting_symlink_destination(tmp_path): thread_uploads_dir = tmp_path / "uploads" thread_uploads_dir.mkdir(parents=True) @@ -1277,10 +1303,11 @@ def test_delete_uploaded_file_removes_owned_conversion_and_preserves_user_markdo @pytest.mark.skipif(os.name == "nt", reason="POSIX legacy filenames are not representable on Windows") -def test_delete_uploaded_file_accepts_listed_legacy_posix_filename(tmp_path): +@pytest.mark.parametrize("filename", ["CON", r"report\draft.pdf", "...", " "]) +def test_delete_uploaded_file_accepts_listed_legacy_posix_filename(tmp_path, filename): thread_uploads_dir = tmp_path / "uploads" thread_uploads_dir.mkdir(parents=True) - legacy = thread_uploads_dir / "CON" + legacy = thread_uploads_dir / filename legacy.write_bytes(b"legacy") with patch.object(uploads, "get_uploads_dir", return_value=thread_uploads_dir): @@ -1293,7 +1320,7 @@ def test_delete_uploaded_file_accepts_listed_legacy_posix_filename(tmp_path): ) ) - assert result == {"success": True, "message": "Deleted CON"} + assert result == {"success": True, "message": f"Deleted {filename}"} assert not legacy.exists() diff --git a/docker/provisioner/README.md b/docker/provisioner/README.md index fd156f808..9d73f2f11 100644 --- a/docker/provisioner/README.md +++ b/docker/provisioner/README.md @@ -1,6 +1,6 @@ # DeerFlow Sandbox Provisioner -The **Sandbox Provisioner** is a FastAPI service that dynamically manages sandbox Pods in Kubernetes. It provides a REST API for the DeerFlow backend to create, monitor, and destroy isolated sandbox environments for code execution. +The **Sandbox Provisioner** is a FastAPI service that dynamically manages sandbox Pods in Kubernetes. It provides a REST API for the DeerFlow backend to create, monitor, and destroy isolated sandbox environments for code execution. Generated upload conversions are passed only as the fixed `/mnt/user-data/.upload-conversions` nested mount; the Provisioner verifies that its source belongs to the requested user/thread and that it is read-only before building either a hostPath or PVC-backed Pod. ## Architecture diff --git a/docker/provisioner/app.py b/docker/provisioner/app.py index 1a6c81367..672e4b17b 100644 --- a/docker/provisioner/app.py +++ b/docker/provisioner/app.py @@ -105,6 +105,7 @@ SAFE_USER_ID_PATTERN = r"^[A-Za-z0-9_\-]+$" DEFAULT_USER_ID = "default" MAX_EXTRA_MOUNTS = 9 ALLOWED_EXTRA_MOUNT_PATHS = { + "/mnt/user-data/.upload-conversions", "/mnt/acp-workspace", "/mnt/skills/custom", "/mnt/skills/integrations", @@ -112,6 +113,8 @@ ALLOWED_EXTRA_MOUNT_PATHS = { "/mnt/integrations/lark-cli/data", "/mnt/integrations/lark-cli/runtime", } +UPLOAD_CONVERSIONS_CONTAINER_PATH = "/mnt/user-data/.upload-conversions" +UPLOAD_CONVERSIONS_DIRNAME = ".upload-conversions" # Path to the kubeconfig *inside* the provisioner container. # Typically the host's ~/.kube/config is mounted here. @@ -175,7 +178,12 @@ def _normalize_extra_mount_container_path(container_path: str) -> str: return normalized -def _validated_extra_mounts(extra_mounts: list["ExtraMount"] | None) -> list["ExtraMount"]: +def _validated_extra_mounts( + extra_mounts: list["ExtraMount"] | None, + *, + thread_id: str | None = None, + user_id: str = DEFAULT_USER_ID, +) -> list["ExtraMount"]: """Validate extra mounts before converting them into K8s hostPath/PVC mounts.""" if not extra_mounts: return [] @@ -193,6 +201,27 @@ def _validated_extra_mounts(extra_mounts: list["ExtraMount"] | None) -> list["Ex raise HTTPException(status_code=400, detail=f"Extra mount host path is outside DeerFlow state: {mount.host_path}") container_path = _normalize_extra_mount_container_path(mount.container_path) + if container_path == UPLOAD_CONVERSIONS_CONTAINER_PATH: + if not mount.read_only: + raise HTTPException(status_code=400, detail="Upload conversion mount must be read-only") + if thread_id is None: + raise HTTPException(status_code=400, detail="Upload conversion mount requires a thread") + expected_host_path = os.path.normpath( + join_host_path( + host_base_dir, + "users", + user_id, + "threads", + thread_id, + "user-data", + UPLOAD_CONVERSIONS_DIRNAME, + ) + ) + if host_path != expected_host_path: + raise HTTPException( + status_code=400, + detail="Upload conversion mount must match the requested user and thread", + ) if container_path in seen_container_paths: raise HTTPException(status_code=400, detail=f"Duplicate extra mount path: {container_path}") seen_container_paths.add(container_path) @@ -259,7 +288,13 @@ def _lark_broker_credential_mounts(extra_mounts: list["ExtraMount"] | None) -> d ``/var/lark/{config,data}`` paths. """ result: dict[str, ExtraMount] = {} - for mount in _validated_extra_mounts(extra_mounts): + credential_candidates = [ + mount + for mount in extra_mounts or [] + if posixpath.normpath(mount.container_path) + in {LARK_CLI_CONFIG_CONTAINER_PATH, LARK_CLI_DATA_CONTAINER_PATH} + ] + for mount in _validated_extra_mounts(credential_candidates): normalized = posixpath.normpath(mount.container_path) if normalized in (LARK_CLI_CONFIG_CONTAINER_PATH, LARK_CLI_DATA_CONTAINER_PATH): result[normalized] = mount @@ -436,9 +471,16 @@ def _sandbox_url(sandbox_id: str, node_port: int | None = None) -> str: return f"http://{NODE_HOST}:{node_port}" -def _build_extra_volumes(extra_mounts: list[ExtraMount] | None = None) -> list[k8s_client.V1Volume]: +def _build_extra_volumes( + extra_mounts: list[ExtraMount] | None = None, + *, + thread_id: str | None = None, + user_id: str = DEFAULT_USER_ID, +) -> list[k8s_client.V1Volume]: volumes: list[k8s_client.V1Volume] = [] - for index, mount in enumerate(_validated_extra_mounts(extra_mounts)): + for index, mount in enumerate( + _validated_extra_mounts(extra_mounts, thread_id=thread_id, user_id=user_id) + ): if USERDATA_PVC_NAME: volumes.append( k8s_client.V1Volume( @@ -462,9 +504,16 @@ def _build_extra_volumes(extra_mounts: list[ExtraMount] | None = None) -> list[k return volumes -def _build_extra_volume_mounts(extra_mounts: list[ExtraMount] | None = None) -> list[k8s_client.V1VolumeMount]: +def _build_extra_volume_mounts( + extra_mounts: list[ExtraMount] | None = None, + *, + thread_id: str | None = None, + user_id: str = DEFAULT_USER_ID, +) -> list[k8s_client.V1VolumeMount]: mounts: list[k8s_client.V1VolumeMount] = [] - for index, mount in enumerate(_validated_extra_mounts(extra_mounts)): + for index, mount in enumerate( + _validated_extra_mounts(extra_mounts, thread_id=thread_id, user_id=user_id) + ): volume_mount = k8s_client.V1VolumeMount( name=_extra_mount_volume_name(index), mount_path=mount.container_path, @@ -578,7 +627,9 @@ def _build_volumes( extra_mounts, provision_lark_cli_runtime=provision_lark_cli_runtime, provision_lark_cli_broker=provision_lark_cli_broker, - ) + ), + thread_id=thread_id, + user_id=user_id, ) ) # The runtime emptyDir is shared by the init container (writer) and the @@ -688,7 +739,9 @@ def _build_volume_mounts( extra_mounts, provision_lark_cli_runtime=provision_lark_cli_runtime, provision_lark_cli_broker=provision_lark_cli_broker, - ) + ), + thread_id=thread_id, + user_id=user_id, ) ) # Sandbox reads the runtime dir (real binary in Pattern A, shim in Pattern B). diff --git a/docs/superpowers/plans/2026-08-06-upload-collision-safety.md b/docs/superpowers/plans/2026-08-06-upload-collision-safety.md index ae3876d4b..fb163d09a 100644 --- a/docs/superpowers/plans/2026-08-06-upload-collision-safety.md +++ b/docs/superpowers/plans/2026-08-06-upload-collision-safety.md @@ -34,7 +34,7 @@ - Modify `backend/packages/harness/deerflow/client.py`: replace direct `shutil.copy2` and request-local naming with shared publication and conversion. - Modify `backend/app/channels/manager.py`: replace scan-then-write inbound attachment persistence with shared publication. - Modify `backend/app/channels/dingtalk.py`: replace the per-instance scan/claim/write sequence with shared publication. -- Modify `backend/packages/harness/deerflow/utils/file_outline.py`: resolve owned conversion paths instead of guessing siblings. +- Modify `backend/packages/harness/deerflow/utils/file_outline.py`: read verified direct Markdown primaries; for every other format resolve owned conversion paths instead of guessing siblings. - Modify upload, router, client, channel, outline, middleware, list-tool, and blocking-I/O tests named in the tasks below. - Modify `README.md`, `backend/AGENTS.md`, `backend/docs/API.md`, `backend/docs/FILE_UPLOAD.md`, `backend/docs/PATH_EXAMPLES.md`, and `backend/docs/rfc-extract-shared-modules.md`: document the invariant and exact paths. @@ -365,7 +365,7 @@ git commit -m "fix: publish uploads without overwriting" - Produces: `replace_system_owned_staged_file(staged: StagedUpload, filename: str) -> Path` for atomic replace only inside the conversion namespace. - Produces: `convert_uploaded_file_to_markdown(upload_path: Path) -> Path | None`. - Changes: `delete_file_safe(base_dir: Path, filename: str) -> dict` deletes the exact owned conversion without an extension-set argument. -- Changes: `extract_outline_for_file(file_path: Path)` reads `conversion_path_for_upload(file_path)` only. +- Changes: `extract_outline_for_file(file_path: Path)` reads a verified regular primary when `file_path` is Markdown; otherwise it reads `conversion_path_for_upload(file_path)` only. - [ ] **Step 1: Write failing conversion ownership, deletion, and outline tests**