From bc8aab0adef8964e8ea3396564c5680d5b45c911 Mon Sep 17 00:00:00 2001 From: Willem Jiang Date: Sat, 23 May 2026 16:31:30 +0800 Subject: [PATCH] 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 | 19 ++++++++++--------- backend/tests/test_uploads_router.py | 5 ++++- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/backend/app/gateway/routers/uploads.py b/backend/app/gateway/routers/uploads.py index 6834ea2f2..9e75a35cd 100644 --- a/backend/app/gateway/routers/uploads.py +++ b/backend/app/gateway/routers/uploads.py @@ -69,7 +69,7 @@ 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) @@ -295,14 +295,15 @@ async def upload_files( _cleanup_uploaded_paths(written_paths) raise HTTPException(status_code=500, detail=f"Failed to upload {file.filename}: {str(e)}") - # When the sandbox uses bind-mounted thread data directories (e.g. AIO with - # LocalContainerBackend), uploaded files are visible inside the container but - # retain the 0o600 permissions set by the gateway. The sandbox process runs - # as a different user and cannot read them. Adjust permissions to add - # group/other read bits so the sandbox can access the files. - if not sync_to_sandbox and getattr(sandbox_provider, "needs_upload_permission_adjustment", True): - for file_path in written_paths: - _make_file_sandbox_readable(file_path) + # 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: diff --git a/backend/tests/test_uploads_router.py b/backend/tests/test_uploads_router.py index b77c54095..46804d321 100644 --- a/backend/tests/test_uploads_router.py +++ b/backend/tests/test_uploads_router.py @@ -236,7 +236,10 @@ def test_upload_files_does_not_adjust_permissions_for_local_sandbox(tmp_path): assert result.success is True make_writable.assert_not_called() - make_readable.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):