fix(sandbox): add group/other read permissions to uploaded files for Docker sandbox (#3127) (#3134)

* fix(sandbox): add group/other read permissions to uploaded files for Docker sandbox (#3127)

  When using AIO sandbox with LocalContainerBackend, uploaded files are
  created with 0o600 (owner-only) permissions by the gateway process
  running as root. The sandbox process inside the Docker container runs
  as a non-root user and cannot read these bind-mounted files, causing
  a "Permission denied" error on read_file.

  Add `needs_upload_permission_adjustment` attribute to SandboxProvider
  (default True) to indicate that uploaded files need chmod adjustment.
  LocalSandboxProvider opts out (same user). A new `_make_file_sandbox_readable`
  function adds S_IRGRP | S_IROTH bits after files are written, changing
  permissions from 0o600 to 0o644 so the sandbox can read the uploads.

  fixes #3127

* fix(uploads): unconditionally adjust file permissions for sandbox access

  The conditional check  meant uploaded files retained 0o600
  permissions in some Docker sandbox configurations, preventing the
  sandbox process (UID 1000) from reading them. Always add group/other
  read bits so every sandbox setup can access uploaded content. Also add
  read bits to the sync-path writable helper as defense in depth.

backend/app/gateway/routers/uploads.py

@@ -69,11 +69,30 @@ def _make_file_sandbox_writable(file_path: os.PathLike[str] | str) -> None:
         logger.warning("Skipping sandbox chmod for symlinked upload path: %s", file_path)
         return
 
-    writable_mode = stat.S_IMODE(file_stat.st_mode) | stat.S_IWUSR | stat.S_IWGRP | stat.S_IWOTH
+    writable_mode = stat.S_IMODE(file_stat.st_mode) | stat.S_IWUSR | stat.S_IWGRP | stat.S_IWOTH | stat.S_IRGRP | stat.S_IROTH
     chmod_kwargs = {"follow_symlinks": False} if os.chmod in os.supports_follow_symlinks else {}
     os.chmod(file_path, writable_mode, **chmod_kwargs)
 
 
+def _make_file_sandbox_readable(file_path: os.PathLike[str] | str) -> None:
+    """Ensure uploaded files are readable by the sandbox process.
+
+    For Docker sandboxes (AIO), the gateway writes files as root with 0o600
+    permissions, then bind-mounts the host directory into the container. The
+    sandbox process inside the container runs as a non-root user and cannot
+    read those files without group/other read bits. This function adds
+    ``S_IRGRP | S_IROTH`` so the sandbox can read the uploaded content.
+    """
+    file_stat = os.lstat(file_path)
+    if stat.S_ISLNK(file_stat.st_mode):
+        logger.warning("Skipping sandbox chmod for symlinked upload path: %s", file_path)
+        return
+
+    readable_mode = stat.S_IMODE(file_stat.st_mode) | stat.S_IRGRP | stat.S_IROTH
+    chmod_kwargs = {"follow_symlinks": False} if os.chmod in os.supports_follow_symlinks else {}
+    os.chmod(file_path, readable_mode, **chmod_kwargs)
+
+
 def _uses_thread_data_mounts(sandbox_provider: SandboxProvider) -> bool:
     return bool(getattr(sandbox_provider, "uses_thread_data_mounts", False))
 
@@ -276,6 +295,16 @@ async def upload_files(
             _cleanup_uploaded_paths(written_paths)
             raise HTTPException(status_code=500, detail=f"Failed to upload {file.filename}: {str(e)}")
 
+    # Uploaded files are created with 0o600 permissions (owner read/write only).
+    # In Docker sandbox deployments the gateway writes as root but the sandbox
+    # process runs as a non-root user (typically UID 1000).  Without group/other
+    # read bits the sandbox cannot access the files — whether the uploads
+    # directory is bind-mounted into the container or synced via
+    # sandbox.update_file.  Always add group/other read bits so every sandbox
+    # configuration can read the uploaded content.
+    for file_path in written_paths:
+        _make_file_sandbox_readable(file_path)
+
     if sync_to_sandbox:
         for file_path, virtual_path in sandbox_sync_targets:
             _make_file_sandbox_writable(file_path)

backend/packages/harness/deerflow/sandbox/local/local_sandbox_provider.py

@@ -63,6 +63,7 @@ class LocalSandboxProvider(SandboxProvider):
     """
 
     uses_thread_data_mounts = True
+    needs_upload_permission_adjustment = False
 
     def __init__(self, max_cached_threads: int = DEFAULT_MAX_CACHED_THREAD_SANDBOXES):
         """Initialize the local sandbox provider with static path mappings.

backend/packages/harness/deerflow/sandbox/sandbox_provider.py

@@ -10,6 +10,7 @@ class SandboxProvider(ABC):
     """Abstract base class for sandbox providers"""
 
     uses_thread_data_mounts: bool = False
+    needs_upload_permission_adjustment: bool = True
 
     @abstractmethod
     def acquire(self, thread_id: str | None = None) -> str:

backend/tests/test_uploads_router.py

@@ -219,6 +219,7 @@ def test_upload_files_does_not_adjust_permissions_for_local_sandbox(tmp_path):
 
     provider = MagicMock()
     provider.uses_thread_data_mounts = True
+    provider.needs_upload_permission_adjustment = False
     provider.acquire.return_value = "local"
     sandbox = MagicMock()
     provider.get.return_value = sandbox
@@ -228,12 +229,17 @@ def test_upload_files_does_not_adjust_permissions_for_local_sandbox(tmp_path):
         patch.object(uploads, "ensure_uploads_dir", return_value=thread_uploads_dir),
         patch.object(uploads, "get_sandbox_provider", return_value=provider),
         patch.object(uploads, "_make_file_sandbox_writable") as make_writable,
+        patch.object(uploads, "_make_file_sandbox_readable") as make_readable,
     ):
         file = UploadFile(filename="notes.txt", file=BytesIO(b"hello uploads"))
         result = asyncio.run(call_unwrapped(uploads.upload_files, "thread-local", request=MagicMock(), files=[file], config=SimpleNamespace()))
 
     assert result.success is True
     make_writable.assert_not_called()
+    # Readable adjustment is now always applied regardless of sandbox type
+    make_readable.assert_called_once()
+    called_path = make_readable.call_args[0][0]
+    assert called_path.name == "notes.txt"
 
 
 def test_upload_files_acquires_non_local_sandbox_before_writing(tmp_path):
@@ -431,6 +437,59 @@ def test_make_file_sandbox_writable_skips_symlinks(tmp_path):
     chmod.assert_not_called()
 
 
+def test_make_file_sandbox_readable_adds_read_bits_for_regular_files(tmp_path):
+    file_path = tmp_path / "data.csv"
+    file_path.write_bytes(b"csv-data")
+    # Simulate the 0o600 permissions set by open_upload_file_no_symlink
+    file_path.chmod(0o600)
+
+    uploads._make_file_sandbox_readable(file_path)
+
+    updated_mode = stat.S_IMODE(file_path.stat().st_mode)
+    assert updated_mode & stat.S_IRUSR
+    assert updated_mode & stat.S_IRGRP
+    assert updated_mode & stat.S_IROTH
+
+
+def test_make_file_sandbox_readable_skips_symlinks(tmp_path):
+    file_path = tmp_path / "target-link.txt"
+    file_path.write_text("hello", encoding="utf-8")
+    symlink_stat = MagicMock(st_mode=stat.S_IFLNK)
+
+    with (
+        patch.object(uploads.os, "lstat", return_value=symlink_stat),
+        patch.object(uploads.os, "chmod") as chmod,
+    ):
+        uploads._make_file_sandbox_readable(file_path)
+
+    chmod.assert_not_called()
+
+
+def test_upload_files_adjusts_read_permissions_for_mounted_non_local_sandbox(tmp_path):
+    thread_uploads_dir = tmp_path / "uploads"
+    thread_uploads_dir.mkdir(parents=True)
+
+    # AIO sandbox with LocalContainerBackend: uses_thread_data_mounts=True
+    # but needs_upload_permission_adjustment=True (default)
+    provider = MagicMock()
+    provider.uses_thread_data_mounts = True
+    provider.needs_upload_permission_adjustment = True
+
+    with (
+        patch.object(uploads, "get_uploads_dir", return_value=thread_uploads_dir),
+        patch.object(uploads, "ensure_uploads_dir", return_value=thread_uploads_dir),
+        patch.object(uploads, "get_sandbox_provider", return_value=provider),
+        patch.object(uploads, "_make_file_sandbox_readable") as make_readable,
+    ):
+        file = UploadFile(filename="notes.txt", file=BytesIO(b"hello uploads"))
+        result = asyncio.run(call_unwrapped(uploads.upload_files, "thread-aio", request=MagicMock(), files=[file], config=SimpleNamespace()))
+
+    assert result.success is True
+    make_readable.assert_called_once()
+    called_path = make_readable.call_args[0][0]
+    assert called_path.name == "notes.txt"
+
+
 def test_upload_files_rejects_dotdot_and_dot_filenames(tmp_path):
     thread_uploads_dir = tmp_path / "uploads"
     thread_uploads_dir.mkdir(parents=True)