From 99fa9f6e2b8430b442e8b9028c91b715309434e3 Mon Sep 17 00:00:00 2001 From: Stergiadis Manos Date: Fri, 23 Jan 2026 19:31:34 +0100 Subject: [PATCH] Fix permissions When the agent issues commands that create directories inside /workspace, those are created by the docker which by default has root userID. So the host cannot access those directories. This causes permission error during cleanup. Even worse, if the agent wants to submit files within those directories then the file transfer also fails. This PR fixes that, --- src/stirrup/tools/code_backends/docker.py | 48 +++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/stirrup/tools/code_backends/docker.py b/src/stirrup/tools/code_backends/docker.py index e566d8b..351d104 100644 --- a/src/stirrup/tools/code_backends/docker.py +++ b/src/stirrup/tools/code_backends/docker.py @@ -379,6 +379,10 @@ async def __aexit__( exc_tb: object, ) -> None: """Stop container and cleanup temp directory.""" + # Fix ownership of all files before cleanup (prevents permission errors on nested directories) + if self._container and self._temp_dir: + await self._fix_file_ownership() + # Stop and remove container if self._container: container = self._container # Capture for lambda type narrowing @@ -623,6 +627,47 @@ async def run_command(self, cmd: str, *, timeout: int = SHELL_TIMEOUT) -> Comman error_kind="execution_error", ) + async def _fix_file_ownership(self, paths: list[str] | None = None) -> None: + """Fix ownership of files created by the container. + + Files and directories created inside the Docker container run as root, + which causes permission issues when trying to move/delete them from the host. + This method runs chown inside the container to fix ownership. + + Args: + paths: Specific paths to fix. If None, fixes all files in working_dir. + Paths should be container paths (absolute or relative to working_dir). + """ + if self._container is None: + return + + try: + # Get the host user ID to chown to + import os + host_uid = os.getuid() + host_gid = os.getgid() + + if paths: + # Fix specific paths + for path in paths: + # Normalize path - handle both relative and absolute + if not path.startswith('/'): + container_path = f"{self._working_dir}/{path}" + else: + container_path = path + + # Use chown -R to handle directories recursively + chown_cmd = f"chown -R {host_uid}:{host_gid} {container_path} 2>/dev/null || true" + await self.run_command(chown_cmd, timeout=10) + else: + # Fix all files in working directory + chown_cmd = f"chown -R {host_uid}:{host_gid} {self._working_dir} 2>/dev/null || true" + await self.run_command(chown_cmd, timeout=10) + + except Exception as exc: + # Don't fail the operation if chown fails, just log warning + logger.warning("Failed to fix file ownership: %s", exc) + async def save_output_files( self, paths: list[str], @@ -662,6 +707,9 @@ async def save_output_files( if dest_env is not None: return await super().save_output_files(paths, output_dir, dest_env) + # Fix ownership of files before moving them (solves permission issues with nested directories) + await self._fix_file_ownership(paths) + # Local filesystem - use optimized move operation output_dir_path = Path(output_dir) output_dir_path.mkdir(parents=True, exist_ok=True)