From e759520cd755a8ad288dd4a4d12cc25d2e4d1d09 Mon Sep 17 00:00:00 2001 From: Vidit Patankar Date: Sun, 31 May 2026 10:07:25 +0530 Subject: [PATCH] fix(sandbox): run post-rename cleanup as the editor's bound user In WorkspaceEditor.apply_operation, an update_file with move_to writes the renamed file as self._user but removed the original path with a bare self._session.rm(destination), dropping the user. Every other session call in the editor threads user=self._user, so on a non-default user the cleanup ran as the wrong identity (permission error mid-rename, or removal under the wrong privileges leaving the original orphaned). Pass user=self._user. Adds a regression test asserting the move cleanup runs as the bound user. Signed-off-by: Vidit Patankar --- src/agents/sandbox/apply_patch.py | 2 +- .../capabilities/test_apply_patch_tool.py | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/agents/sandbox/apply_patch.py b/src/agents/sandbox/apply_patch.py index d85598f487..0262a64fed 100644 --- a/src/agents/sandbox/apply_patch.py +++ b/src/agents/sandbox/apply_patch.py @@ -97,7 +97,7 @@ async def apply_operation( moved_destination = self._session.normalize_path(moved_relative_path) await self._write_text(moved_destination, updated_text) if moved_destination != destination: - await self._session.rm(destination) + await self._session.rm(destination, user=self._user) moved_display_path = moved_relative_path.as_posix() return ApplyPatchResult( output=f"Updated {display_path}\nMoved {display_path} to {moved_display_path}" diff --git a/tests/sandbox/capabilities/test_apply_patch_tool.py b/tests/sandbox/capabilities/test_apply_patch_tool.py index bebb821213..3c70d338d3 100644 --- a/tests/sandbox/capabilities/test_apply_patch_tool.py +++ b/tests/sandbox/capabilities/test_apply_patch_tool.py @@ -177,6 +177,31 @@ async def test_editor_runs_file_operations_as_bound_user(self) -> None: assert session.write_users == ["sandbox-user", "sandbox-user"] assert session.rm_users == ["sandbox-user"] + @pytest.mark.asyncio + async def test_editor_runs_move_cleanup_as_bound_user(self) -> None: + session = UserRecordingApplyPatchSession() + session.files[Path("/workspace/existing.txt")] = b"old\n" + tool = SandboxApplyPatchTool(session=session, user=User(name="sandbox-user")) + + await cast( + Awaitable[ApplyPatchResult], + tool.editor.update_file( + ApplyPatchOperation( + type="update_file", + path="existing.txt", + diff="@@\n-old\n+new\n", + move_to="renamed.txt", + ) + ), + ) + + # Renaming writes the new file and removes the original; both must run + # as the editor's bound user, not the default sandbox user. + assert session.write_users == ["sandbox-user"] + assert session.rm_users == ["sandbox-user"] + assert Path("/workspace/existing.txt") not in session.files + assert Path("/workspace/renamed.txt") in session.files + @pytest.mark.asyncio async def test_custom_tool_input_create_update_move_delete(self) -> None: session = ApplyPatchSession()