mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-25 14:06:18 +00:00
* fix(channels): fix Telegram inbound file download and sandbox readability - Shut down the download Bot on the Telegram loop (Bot.shutdown(), which closes its HTTPX clients) before the loop stops, instead of the non-existent Bot.session.close(). - Grant group/other read on channel-downloaded uploads so the non-root AIO/Docker sandbox process can read the root-written 0o600 file. - Apply the sandbox permission change with os.fchmod on a descriptor opened with O_NOFOLLOW (validated as a regular file via fstat), bound to the validated upload inode, so a symlink swapped in after lstat cannot redirect the chmod to a target outside the uploads directory. The open also uses O_NONBLOCK so a sandbox-swapped FIFO cannot block the read-only open before the regular-file check (matching the existing open_upload_file_no_symlink convention). Centralized in a shared apply_upload_sandbox_permits helper reused by the channel inbound path and the HTTP upload readable/writable helpers. - Surface the download failure cause chain in logs with the Bot API URL masked: the configured token is redacted and both URL forms are collapsed, covering the file download URL (/file/bot<token>/...) and the method URLs (/bot<token>/getMe, /bot<token>/getFile). - Migrate the existing receive_file tests onto the download Bot and add coverage for _get_download_bot (loop-bound creation + caching, cleanup on init failure), download-bot routing over the application bot, real-Bot shutdown closing both HTTPX clients, receive_file timeout-containment, masked cause-chain logging (file and method URL forms), and inbound-file sandbox perms (including the swap-after-lstat symlink regression). * fix(uploads): surface sandbox permission failures --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
448 lines
18 KiB
Python
448 lines
18 KiB
Python
"""Tests for deerflow.uploads.manager — shared upload management logic."""
|
||
|
||
import errno
|
||
import os
|
||
import shutil
|
||
import stat
|
||
from unittest.mock import patch
|
||
|
||
import pytest
|
||
|
||
from deerflow.uploads.manager import (
|
||
PathTraversalError,
|
||
UnsafeUploadPathError,
|
||
apply_upload_sandbox_permits,
|
||
claim_unique_filename,
|
||
cleanup_stale_upload_staging_files,
|
||
copy_upload_file_no_symlink,
|
||
delete_file_safe,
|
||
list_files_in_dir,
|
||
normalize_filename,
|
||
validate_path_traversal,
|
||
write_upload_file_no_symlink,
|
||
)
|
||
|
||
|
||
@pytest.mark.skipif(not (hasattr(os, "O_NOFOLLOW") and hasattr(os, "fchmod")), reason="POSIX-only: O_NOFOLLOW + fchmod")
|
||
def test_apply_upload_sandbox_permits_propagates_permission_errors(tmp_path):
|
||
upload = tmp_path / "attachment.bin"
|
||
upload.write_bytes(b"attachment")
|
||
upload.chmod(0o600)
|
||
|
||
with patch.object(os, "fchmod", side_effect=PermissionError("permission denied")):
|
||
with pytest.raises(PermissionError, match="permission denied"):
|
||
apply_upload_sandbox_permits(upload, stat.S_IRGRP | stat.S_IROTH)
|
||
|
||
assert stat.S_IMODE(upload.stat().st_mode) == 0o600
|
||
|
||
|
||
def test_apply_upload_sandbox_permits_fallback_propagates_permission_errors(tmp_path, monkeypatch):
|
||
upload = tmp_path / "attachment.bin"
|
||
upload.write_bytes(b"attachment")
|
||
upload.chmod(0o600)
|
||
monkeypatch.delattr(os, "O_NOFOLLOW", raising=False)
|
||
|
||
with patch.object(os, "chmod", side_effect=PermissionError("permission denied")):
|
||
with pytest.raises(PermissionError, match="permission denied"):
|
||
apply_upload_sandbox_permits(upload, stat.S_IRGRP | stat.S_IROTH)
|
||
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# normalize_filename
|
||
# ---------------------------------------------------------------------------
|
||
|
||
|
||
class TestNormalizeFilename:
|
||
def test_safe_filename(self):
|
||
assert normalize_filename("report.pdf") == "report.pdf"
|
||
|
||
def test_strips_path_components(self):
|
||
assert normalize_filename("../../etc/passwd") == "passwd"
|
||
|
||
def test_rejects_empty(self):
|
||
with pytest.raises(ValueError, match="empty"):
|
||
normalize_filename("")
|
||
|
||
def test_rejects_dot_dot(self):
|
||
with pytest.raises(ValueError, match="unsafe"):
|
||
normalize_filename("..")
|
||
|
||
def test_strips_separators(self):
|
||
assert normalize_filename("path/to/file.txt") == "file.txt"
|
||
|
||
def test_dot_only(self):
|
||
with pytest.raises(ValueError, match="unsafe"):
|
||
normalize_filename(".")
|
||
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# claim_unique_filename
|
||
# ---------------------------------------------------------------------------
|
||
|
||
|
||
class TestDeduplicateFilename:
|
||
def test_no_collision(self):
|
||
seen: set[str] = set()
|
||
assert claim_unique_filename("data.txt", seen) == "data.txt"
|
||
assert "data.txt" in seen
|
||
|
||
def test_single_collision(self):
|
||
seen = {"data.txt"}
|
||
assert claim_unique_filename("data.txt", seen) == "data_1.txt"
|
||
assert "data_1.txt" in seen
|
||
|
||
def test_triple_collision(self):
|
||
seen = {"data.txt", "data_1.txt", "data_2.txt"}
|
||
assert claim_unique_filename("data.txt", seen) == "data_3.txt"
|
||
assert "data_3.txt" in seen
|
||
|
||
def test_mutates_seen(self):
|
||
seen: set[str] = set()
|
||
claim_unique_filename("a.txt", seen)
|
||
claim_unique_filename("a.txt", seen)
|
||
assert seen == {"a.txt", "a_1.txt"}
|
||
|
||
def test_max_length_name_stays_within_filename_limit(self):
|
||
# A 255-byte name passes normalize_filename; the deduplicated name
|
||
# must not exceed that limit, or the write path rejects it.
|
||
name = "a" * 251 + ".txt"
|
||
seen = {name}
|
||
deduped = claim_unique_filename(name, seen)
|
||
assert deduped != name
|
||
assert deduped.endswith("_1.txt")
|
||
assert len(deduped.encode("utf-8")) <= 255
|
||
# The truncated result must round-trip through normalize_filename.
|
||
assert normalize_filename(deduped) == deduped
|
||
|
||
def test_max_length_collisions_stay_unique_across_truncation(self):
|
||
name = "a" * 251 + ".txt"
|
||
seen = {name}
|
||
first = claim_unique_filename(name, seen)
|
||
second = claim_unique_filename(name, seen)
|
||
assert first != second
|
||
assert len(second.encode("utf-8")) <= 255
|
||
|
||
def test_multibyte_stem_is_truncated_on_a_codepoint_boundary(self):
|
||
# 85 CJK chars × 3 bytes = 255 bytes.
|
||
name = "深" * 85
|
||
seen = {name}
|
||
deduped = claim_unique_filename(name, seen)
|
||
assert len(deduped.encode("utf-8")) <= 255
|
||
assert deduped.endswith("_1")
|
||
# No replacement characters / decode artifacts.
|
||
deduped.encode("utf-8").decode("utf-8")
|
||
|
||
def test_short_names_keep_existing_dedupe_shape(self):
|
||
seen = {"data.txt"}
|
||
assert claim_unique_filename("data.txt", seen) == "data_1.txt"
|
||
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# validate_path_traversal
|
||
# ---------------------------------------------------------------------------
|
||
|
||
|
||
class TestValidatePathTraversal:
|
||
def test_inside_base_ok(self, tmp_path):
|
||
child = tmp_path / "file.txt"
|
||
child.touch()
|
||
validate_path_traversal(child, tmp_path) # no exception
|
||
|
||
def test_outside_base_raises(self, tmp_path):
|
||
outside = tmp_path / ".." / "evil.txt"
|
||
with pytest.raises(PathTraversalError, match="traversal"):
|
||
validate_path_traversal(outside, tmp_path)
|
||
|
||
def test_symlink_escape(self, tmp_path):
|
||
target = tmp_path.parent / "secret.txt"
|
||
target.touch()
|
||
link = tmp_path / "escape"
|
||
try:
|
||
link.symlink_to(target)
|
||
except OSError as exc:
|
||
if getattr(exc, "winerror", None) == 1314:
|
||
pytest.skip("symlink creation requires Developer Mode or elevated privileges on Windows")
|
||
raise
|
||
with pytest.raises(PathTraversalError, match="traversal"):
|
||
validate_path_traversal(link, tmp_path)
|
||
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# write_upload_file_no_symlink
|
||
# ---------------------------------------------------------------------------
|
||
|
||
|
||
class TestWriteUploadFileNoSymlink:
|
||
def test_writes_new_file(self, tmp_path):
|
||
dest = write_upload_file_no_symlink(tmp_path, "notes.txt", b"hello")
|
||
|
||
assert dest == tmp_path / "notes.txt"
|
||
assert dest.read_bytes() == b"hello"
|
||
|
||
def test_overwrites_existing_regular_file_with_single_link(self, tmp_path):
|
||
dest = tmp_path / "notes.txt"
|
||
dest.write_bytes(b"old contents")
|
||
assert os.stat(dest).st_nlink == 1
|
||
|
||
result = write_upload_file_no_symlink(tmp_path, "notes.txt", b"new contents")
|
||
|
||
assert result == dest
|
||
assert dest.read_bytes() == b"new contents"
|
||
assert os.stat(dest).st_nlink == 1
|
||
|
||
def test_fallback_without_no_follow_support_succeeds(self, tmp_path, monkeypatch):
|
||
monkeypatch.delattr(os, "O_NOFOLLOW", raising=False)
|
||
|
||
# When O_NOFOLLOW is absent (Windows), the function falls back to
|
||
# a dual-lstat + fstat approach and succeeds.
|
||
result = write_upload_file_no_symlink(tmp_path, "notes.txt", b"hello")
|
||
assert result == tmp_path / "notes.txt"
|
||
assert (tmp_path / "notes.txt").read_bytes() == b"hello"
|
||
|
||
def test_open_uses_nonblocking_flag_when_available(self, tmp_path):
|
||
if not hasattr(os, "O_NONBLOCK"):
|
||
pytest.skip("O_NONBLOCK not available on this platform")
|
||
with patch("deerflow.uploads.manager.os.open", side_effect=OSError(errno.ENXIO, "no reader")) as open_mock:
|
||
with pytest.raises(UnsafeUploadPathError, match="Unsafe upload destination"):
|
||
write_upload_file_no_symlink(tmp_path, "pipe.txt", b"hello")
|
||
|
||
flags = open_mock.call_args.args[1]
|
||
assert flags & os.O_NONBLOCK
|
||
|
||
@pytest.mark.parametrize("open_errno", [errno.ENXIO, errno.EAGAIN])
|
||
def test_nonblocking_special_file_open_errors_are_unsafe(self, tmp_path, open_errno):
|
||
if not hasattr(os, "O_NONBLOCK"):
|
||
pytest.skip("O_NONBLOCK not available on this platform")
|
||
with patch("deerflow.uploads.manager.os.open", side_effect=OSError(open_errno, "would block")):
|
||
with pytest.raises(UnsafeUploadPathError, match="Unsafe upload destination"):
|
||
write_upload_file_no_symlink(tmp_path, "pipe.txt", b"hello")
|
||
|
||
assert not (tmp_path / "pipe.txt").exists()
|
||
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# copy_upload_file_no_symlink
|
||
# ---------------------------------------------------------------------------
|
||
|
||
|
||
class TestCopyUploadFileNoSymlink:
|
||
def test_copies_content_mode_and_timestamps(self, tmp_path):
|
||
uploads = tmp_path / "uploads"
|
||
uploads.mkdir()
|
||
src = tmp_path / "notes.txt"
|
||
src.write_bytes(b"hello")
|
||
os.chmod(src, 0o640)
|
||
os.utime(src, ns=(1_700_000_000_000_000_000, 1_700_000_000_000_000_000))
|
||
|
||
dest = copy_upload_file_no_symlink(uploads, "notes.txt", src)
|
||
|
||
assert dest == uploads / "notes.txt"
|
||
assert dest.read_bytes() == b"hello"
|
||
if os.chmod in os.supports_fd:
|
||
assert stat.S_IMODE(os.stat(dest).st_mode) == 0o640
|
||
if os.utime in os.supports_fd:
|
||
assert os.stat(dest).st_mtime_ns == 1_700_000_000_000_000_000
|
||
|
||
def test_overwrites_existing_regular_file(self, tmp_path):
|
||
uploads = tmp_path / "uploads"
|
||
uploads.mkdir()
|
||
(uploads / "notes.txt").write_bytes(b"old contents")
|
||
src = tmp_path / "notes.txt"
|
||
src.write_bytes(b"new contents")
|
||
|
||
dest = copy_upload_file_no_symlink(uploads, "notes.txt", src)
|
||
|
||
assert dest.read_bytes() == b"new contents"
|
||
|
||
def test_rejects_symlink_destination(self, tmp_path):
|
||
uploads = tmp_path / "uploads"
|
||
uploads.mkdir()
|
||
outside = tmp_path / "outside.txt"
|
||
outside.write_bytes(b"original")
|
||
link = uploads / "notes.txt"
|
||
try:
|
||
link.symlink_to(outside)
|
||
except OSError as exc:
|
||
if getattr(exc, "winerror", None) == 1314:
|
||
pytest.skip("symlink creation requires Developer Mode or elevated privileges on Windows")
|
||
raise
|
||
src = tmp_path / "notes.txt"
|
||
src.write_bytes(b"attacker-chosen target")
|
||
|
||
with pytest.raises(UnsafeUploadPathError):
|
||
copy_upload_file_no_symlink(uploads, "notes.txt", src)
|
||
|
||
assert outside.read_bytes() == b"original"
|
||
assert link.is_symlink()
|
||
|
||
def test_rejects_copying_a_file_onto_itself(self, tmp_path):
|
||
uploads = tmp_path / "uploads"
|
||
uploads.mkdir()
|
||
src = uploads / "notes.txt"
|
||
src.write_bytes(b"IMPORTANT")
|
||
|
||
with pytest.raises(shutil.SameFileError):
|
||
copy_upload_file_no_symlink(uploads, "notes.txt", src)
|
||
|
||
assert src.read_bytes() == b"IMPORTANT"
|
||
|
||
def test_rejects_a_hardlink_to_the_destination(self, tmp_path):
|
||
"""Identity, not path text: another name for the same inode is the same file."""
|
||
uploads = tmp_path / "uploads"
|
||
uploads.mkdir()
|
||
dest = uploads / "notes.txt"
|
||
dest.write_bytes(b"IMPORTANT")
|
||
src = uploads / "same-inode.txt"
|
||
try:
|
||
os.link(dest, src)
|
||
except (OSError, NotImplementedError) as exc:
|
||
pytest.skip(f"hardlinks unavailable on this platform: {exc}")
|
||
|
||
with pytest.raises(shutil.SameFileError):
|
||
copy_upload_file_no_symlink(uploads, "notes.txt", src)
|
||
|
||
assert dest.read_bytes() == b"IMPORTANT"
|
||
|
||
def test_missing_source_leaves_existing_destination_untouched(self, tmp_path):
|
||
uploads = tmp_path / "uploads"
|
||
uploads.mkdir()
|
||
(uploads / "notes.txt").write_bytes(b"keep me")
|
||
|
||
with pytest.raises(FileNotFoundError):
|
||
copy_upload_file_no_symlink(uploads, "notes.txt", tmp_path / "missing.txt")
|
||
|
||
assert (uploads / "notes.txt").read_bytes() == b"keep me"
|
||
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# list_files_in_dir
|
||
# ---------------------------------------------------------------------------
|
||
|
||
|
||
class TestListFilesInDir:
|
||
def test_empty_dir(self, tmp_path):
|
||
result = list_files_in_dir(tmp_path)
|
||
assert result == {"files": [], "count": 0}
|
||
|
||
def test_nonexistent_dir(self, tmp_path):
|
||
result = list_files_in_dir(tmp_path / "nope")
|
||
assert result == {"files": [], "count": 0}
|
||
|
||
def test_multiple_files_sorted(self, tmp_path):
|
||
(tmp_path / "b.txt").write_text("b")
|
||
(tmp_path / "a.txt").write_text("a")
|
||
result = list_files_in_dir(tmp_path)
|
||
assert result["count"] == 2
|
||
assert result["files"][0]["filename"] == "a.txt"
|
||
assert result["files"][1]["filename"] == "b.txt"
|
||
for f in result["files"]:
|
||
assert set(f.keys()) == {"filename", "size", "path", "extension", "modified"}
|
||
|
||
def test_ignores_subdirectories(self, tmp_path):
|
||
(tmp_path / "file.txt").write_text("data")
|
||
(tmp_path / "subdir").mkdir()
|
||
result = list_files_in_dir(tmp_path)
|
||
assert result["count"] == 1
|
||
assert result["files"][0]["filename"] == "file.txt"
|
||
|
||
def test_filters_only_upload_staging_files(self, tmp_path):
|
||
(tmp_path / ".env").write_text("intentional dotfile")
|
||
(tmp_path / ".upload-active.part").write_text("partial")
|
||
(tmp_path / ".upload-note.txt").write_text("intentional upload")
|
||
(tmp_path / "draft.part").write_text("intentional upload")
|
||
(tmp_path / "visible.txt").write_text("visible")
|
||
|
||
result = list_files_in_dir(tmp_path)
|
||
|
||
assert result["count"] == 4
|
||
assert [f["filename"] for f in result["files"]] == [".env", ".upload-note.txt", "draft.part", "visible.txt"]
|
||
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# cleanup_stale_upload_staging_files
|
||
# ---------------------------------------------------------------------------
|
||
|
||
|
||
class TestCleanupStaleUploadStagingFiles:
|
||
def test_removes_only_stale_staging_files_from_all_upload_layouts(self, tmp_path):
|
||
legacy_uploads = tmp_path / "threads" / "thread-legacy" / "user-data" / "uploads"
|
||
user_uploads = tmp_path / "users" / "owner-1" / "threads" / "thread-owned" / "user-data" / "uploads"
|
||
unrelated_uploads = tmp_path / "misc" / "thread-other" / "user-data" / "uploads"
|
||
for uploads_dir in (legacy_uploads, user_uploads, unrelated_uploads):
|
||
uploads_dir.mkdir(parents=True)
|
||
|
||
(legacy_uploads / ".upload-old.part").write_text("legacy partial")
|
||
(user_uploads / ".upload-new.part").write_text("user partial")
|
||
(unrelated_uploads / ".upload-ignore.part").write_text("outside layout")
|
||
(legacy_uploads / ".env").write_text("intentional dotfile")
|
||
(legacy_uploads / ".upload-note.txt").write_text("intentional upload")
|
||
(legacy_uploads / "draft.part").write_text("intentional upload")
|
||
|
||
removed = cleanup_stale_upload_staging_files(tmp_path)
|
||
|
||
assert removed == 2
|
||
assert not (legacy_uploads / ".upload-old.part").exists()
|
||
assert not (user_uploads / ".upload-new.part").exists()
|
||
assert (unrelated_uploads / ".upload-ignore.part").exists()
|
||
assert (legacy_uploads / ".env").exists()
|
||
assert (legacy_uploads / ".upload-note.txt").exists()
|
||
assert (legacy_uploads / "draft.part").exists()
|
||
|
||
|
||
# ---------------------------------------------------------------------------
|
||
# delete_file_safe
|
||
# ---------------------------------------------------------------------------
|
||
|
||
|
||
class TestDeleteFileSafe:
|
||
def test_delete_existing_file(self, tmp_path):
|
||
f = tmp_path / "test.txt"
|
||
f.write_text("data")
|
||
result = delete_file_safe(tmp_path, "test.txt")
|
||
assert result["success"] is True
|
||
assert not f.exists()
|
||
|
||
def test_delete_nonexistent_raises(self, tmp_path):
|
||
with pytest.raises(FileNotFoundError):
|
||
delete_file_safe(tmp_path, "nope.txt")
|
||
|
||
def test_delete_traversal_raises(self, tmp_path):
|
||
with pytest.raises(PathTraversalError, match="traversal"):
|
||
delete_file_safe(tmp_path, "../outside.txt")
|
||
|
||
def test_delete_keeps_the_converted_markdown(self, tmp_path):
|
||
"""Companion ownership cannot be proven from the name, so nothing is guessed at."""
|
||
(tmp_path / "a.docx").write_bytes(b"DOCX")
|
||
(tmp_path / "a.md").write_text("converted from the docx", encoding="utf-8")
|
||
(tmp_path / "a.pdf").write_bytes(b"PDF")
|
||
(tmp_path / "a_1.md").write_text("converted from the pdf", encoding="utf-8")
|
||
|
||
result = delete_file_safe(tmp_path, "a.pdf")
|
||
|
||
assert result["success"] is True
|
||
assert not (tmp_path / "a.pdf").exists()
|
||
# a.md belongs to a.docx; deleting a.pdf used to remove it.
|
||
assert (tmp_path / "a.md").read_text(encoding="utf-8") == "converted from the docx"
|
||
assert (tmp_path / "a_1.md").read_text(encoding="utf-8") == "converted from the pdf"
|
||
|
||
def test_delete_symlink_to_sibling_upload_keeps_target(self, tmp_path):
|
||
"""A symlink planted in the uploads dir must not delete the upload it aliases."""
|
||
victim = tmp_path / "victim.pdf"
|
||
victim.write_bytes(b"pdf-bytes")
|
||
companion = tmp_path / "victim.md"
|
||
companion.write_text("converted", encoding="utf-8")
|
||
alias = tmp_path / "alias.pdf"
|
||
try:
|
||
alias.symlink_to(victim.name)
|
||
except OSError as exc:
|
||
if getattr(exc, "winerror", None) == 1314:
|
||
pytest.skip("symlink creation requires Developer Mode or elevated privileges on Windows")
|
||
raise
|
||
|
||
with pytest.raises(FileNotFoundError):
|
||
delete_file_safe(tmp_path, "alias.pdf")
|
||
|
||
assert victim.read_bytes() == b"pdf-bytes"
|
||
assert companion.exists()
|
||
assert alias.is_symlink()
|