From 22623d1b00041ba4047a29767af63ba5e640a1eb Mon Sep 17 00:00:00 2001 From: JasonH Date: Mon, 5 Oct 2026 18:38:22 +0800 Subject: [PATCH] fix(projects): validate explicit document names before staging (#6287) --- README.md | 5 ++++ backend/app/gateway/AGENTS.md | 2 +- .../harness/deerflow/projects/documents.py | 14 ++--------- .../tests/test_project_documents_promotion.py | 23 +++++++++++++++++++ 4 files changed, 31 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 1fa12a450e..37353b525a 100644 --- a/README.md +++ b/README.md @@ -2334,6 +2334,11 @@ from the project page's Documents section: - **Attach to thread**: copy a shelf file into a thread's uploads through the normal ingestion pipeline, so the conversation can work with it directly. +New shelf names, including explicit upload `name` and promotion `shelf_name`, +follow ordinary upload filename validation. Names containing NUL, Windows +reserved device names (such as `CON.txt`), or trailing dots are rejected with +`400` before bytes are staged. + Runs on member threads also receive a bounded `` index rendered per run from the pinned snapshot (capped by `projects.shelf_index_max_entries` and `projects.shelf_index_max_bytes`), and the agent can page the shelf and read diff --git a/backend/app/gateway/AGENTS.md b/backend/app/gateway/AGENTS.md index b8f1422882..aca1b3a7ab 100644 --- a/backend/app/gateway/AGENTS.md +++ b/backend/app/gateway/AGENTS.md @@ -117,7 +117,7 @@ owner-scoped assistant version selection remains enabled. | **Subagents** (`/api/subagents`) | Admin managed-worker CRUD and listing. | | **Integrations** (`/api/integrations`) | `GET /lark/status` - inspect managed Lark/Feishu CLI integration state, including `sandbox_runtime_mode` / `sandbox_runtime_probed` / `sandbox_runtime_ready` (whether `lark-cli` will be present in the sandbox at chat time); `POST /lark/install` - admin-only install of the official `lark-*` managed skill pack; `POST /lark/config/start` and `/lark/config/complete` - internal first-time Lark connection setup; `POST /lark/config/credentials` - atomically switch the caller's per-user Lark app after validating the new `app_id`/`app_secret` through the official CLI's live tenant-token probe, revoke/remove the previous OAuth tokens, and restore the prior credential tree if the switch fails; `POST /lark/auth/start` and `/lark/auth/complete` - browser device-flow user authorization without terminal access, with optional `domains` / exact `scope` for incremental permission grants. Config and auth flows carry a server-issued, per-user generation persisted under the credential lock; a rejected direct switch leaves the current generation unchanged, stale completions return 409, and browser re-registration uses the same token-clearing/revocation transaction as direct credential switches. | | **Memory** (`/api/memory`) | `GET /` - memory data; `POST /reload` - force reload; `GET /config` - config; `GET /status` - config + data | -| **Uploads** (`/api/threads/{id}/uploads`) | `POST /` - upload files (auto-converts PDF/PPT/Excel/Word); non-mounted sync releases its lease after the last holder exits; `GET /list` - list; `DELETE /{filename}` - delete a regular file; symlinks 404 and a converted `.md` is kept (#5672) | +| **Uploads** (`/api/threads/{id}/uploads`) | `POST /` - upload/convert PDF/PPT/Excel/Word; upload/shelf names share `normalize_filename` before staging; non-mounted sync lease ends when its last holder exits; `GET /list`; `DELETE /{filename}` - regular files, symlinks 404, keeps converted `.md` (#5672) | | **Threads** (`/api/threads/{id}`) | `DELETE /` - remove DeerFlow-managed local thread data after LangGraph thread deletion; `POST /branches` - branch a completed assistant turn with a replay checkpoint; inherited titles take next-free displayed sibling suffixes, including explicit/renamed ones, while explicit titles stay unchanged. Durable `branch` admission rejects races. Workspace files are not checkpointed, so the branch only best-effort copies the current workspace when branching from the **latest** turn (`workspace_clone_mode="current_thread_best_effort"`); branching from an older/historical turn skips the copy (`workspace_clone_mode="skipped_historical_turn"`) so the branch never inherits files that only exist in a later timeline. Thread-scoped runtime channels (`sandbox`, `thread_data`) are not copied onto the branch: the parent's `sandbox_id` binds path mappings and the release lifecycle to the parent's workspace, so the branch lazily acquires its own sandbox instead. Branch creation also seeds the new thread's run-event feed from the branch checkpoint's visible messages (`history_seed_mode` in the response): the thread feed reads run_events, not checkpoints, so without the seed the inherited history disappears from the UI after the branch's first run (#4380). Seeded rows are grouped into one synthetic run per inherited turn (`branch-seed-{thread_id}-{n}`, a new turn opening at every persisted human message, including an allowlisted hidden `ask_clarification` reply) because `run_id` is a turn identity to the feed's consumers, not a provenance tag: regenerating an inherited answer supersedes that row's whole `run_id` in `GET /messages/page`, so one shared id for the entire seed deleted the complete inherited history on a branch's first regenerate (#4458); `GET /goal`, `PUT /goal`, `DELETE /goal` - read, set, and clear the active thread goal; `POST /compact` - summarize older active context, deriving memory policy and bucket from the state-producing checkpoint rather than request `agent_name`, and block while a run is in flight; unexpected failures return a generic 500 detail | | **Artifacts** (`/api/threads/{id}/artifacts`) | GET streams byte ranges; HTML/XML/XSL/`+xml` (also `.skill` members) and `?download=true` force attachments. PUT atomically replaces existing UTF-8 outputs on matching SHA-256; active runs conflict. Non-mounted sync uses a request lease and drains remote/local commit-or-rollback across cancellation before releasing reservations/leases. `resolve_outputs_confined_path` normalizes `..` and confines host paths against traversal/symlinks. Preserve POSIX ownership/mode where supported; Windows keeps temp-file permissions. Digest cache: path/dev/ino/ctime_ns/mtime_ns/size; identity invalidates same-size/mtime replacements. Large regular files use stat-prefixed ETags; If-Range requires matching ETags. Windows ctime is creation time. Keep cache reuse, size limits and worker hashing. | | **Suggestions** (`/api/suggestions`) | `GET /config` - returns global suggestions config boolean; `POST /threads/{id}/suggestions` - generate follow-up questions; rich list/block model content is normalized and inline reasoning (`...`, including unclosed/truncated blocks from reasoning models like MiniMax-M3) is stripped before JSON parsing | diff --git a/backend/packages/harness/deerflow/projects/documents.py b/backend/packages/harness/deerflow/projects/documents.py index e3554ff514..2bec301d01 100644 --- a/backend/packages/harness/deerflow/projects/documents.py +++ b/backend/packages/harness/deerflow/projects/documents.py @@ -37,7 +37,7 @@ from pathlib import Path from typing import TYPE_CHECKING, Any from deerflow.config.paths import Paths -from deerflow.uploads.manager import is_reserved_upload_filename +from deerflow.uploads.manager import normalize_filename from deerflow.utils.file_conversion import CONVERTIBLE_EXTENSIONS, convert_file_to_markdown from deerflow.utils.file_io import await_drained, run_file_io from deerflow.utils.text_detection import is_text_file_by_content @@ -47,8 +47,6 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) -_MAX_FILENAME_BYTES = 255 - class ShelfUploadTooLargeError(Exception): """Raised when staged bytes exceed ``uploads.max_file_size`` (mapped to 413).""" @@ -64,17 +62,9 @@ def validate_shelf_filename(name: str) -> str: including Win32 aliases; existing shelf rows are not revalidated on reads. """ candidate = name.strip() if name else "" - if not candidate: - raise ValueError("Filename is empty") if "/" in candidate or "\\" in candidate: raise ValueError(f"Filename contains a path separator: {name!r}") - if candidate in {".", ".."}: - raise ValueError(f"Filename is unsafe: {name!r}") - if len(candidate.encode("utf-8")) > _MAX_FILENAME_BYTES: - raise ValueError(f"Filename exceeds {_MAX_FILENAME_BYTES} UTF-8 bytes") - if is_reserved_upload_filename(candidate): - raise ValueError(f"Filename uses reserved upload staging pattern: {name!r}") - return candidate + return normalize_filename(candidate) def shelf_relpath(project_id: str, sha256: str, document_id: str) -> str: diff --git a/backend/tests/test_project_documents_promotion.py b/backend/tests/test_project_documents_promotion.py index 421f66aabf..090da2575e 100644 --- a/backend/tests/test_project_documents_promotion.py +++ b/backend/tests/test_project_documents_promotion.py @@ -203,6 +203,29 @@ class TestFromThread: assert source.read_bytes() == b"user document" assert client.get(f"/api/projects/{pid}/documents").json()["total"] == 0 + @pytest.mark.parametrize("filename", ["report\x00.txt", "CON.txt", "com³.txt", "report.txt."]) + def test_invalid_explicit_name_rejected_before_shelf_upload(self, tmp_path, filename): + app = _build_app(tmp_path) + with TestClient(app, raise_server_exceptions=False) as client: + pid = _create_project(client)["id"] + response = client.post(f"/api/projects/{pid}/documents", files={"file": ("safe.txt", b"user document")}, data={"name": filename}) + assert response.status_code == 400 + assert client.get(f"/api/projects/{pid}/documents").json()["total"] == 0 + assert not (get_paths().project_documents_dir(_USER, pid) / ".staging").exists() + + @pytest.mark.parametrize("filename", ["report\x00.txt", "CON.txt", "com³.txt", "report.txt."]) + def test_invalid_explicit_name_rejected_before_shelf_promotion(self, tmp_path, filename): + app = _build_app(tmp_path) + with TestClient(app, raise_server_exceptions=False) as client: + pid = _create_project(client)["id"] + _seed_thread(app, "thread-1") + source = _thread_file("thread-1", "output", "notes.txt", b"user document") + response = _promote(client, pid, thread_id="thread-1", kind="output", name="notes.txt", shelf_name=filename) + assert response.status_code == 400 + assert source.read_bytes() == b"user document" + assert client.get(f"/api/projects/{pid}/documents").json()["total"] == 0 + assert not (get_paths().project_documents_dir(_USER, pid) / ".staging").exists() + def test_promote_upload_with_default_name_records_provenance(self, tmp_path): app = _build_app(tmp_path) with TestClient(app) as client: