fix: workspace folder permission & app container cannot access client API (#3300)

* also copy over pyproject and poetry lock

* add missing readme

* remove extra git config init since it is already done in client.py

* only chown if the /workspace dir does not exists

* Revert "remove extra git config init since it is already done in client.py"

This reverts commit e8556cd.

* remove extra git config init since it is already done in client.py

* fix test runtime

* print container log while reconnecting

* print log in more readable format

* print log in more readable format

* increase lines

* clean up sandbox and ssh related stuff

* remove ssh hostname

* remove ssh hostname

* fix docker app cannot access runtime API issue

* remove ssh password

* API HOSTNAME should be pre-fixed with SANDBOX

* update config

* fix typo that breaks the test

Author: xingyaoww

containers/app/Dockerfile

@@ -37,6 +37,7 @@ ARG OPEN_DEVIN_BUILD_VERSION #re-declare for this section
 ENV RUN_AS_DEVIN=true
 # A random number--we need this to be different from the user's UID on the host machine
 ENV OPENDEVIN_USER_ID=42420
+ENV SANDBOX_API_HOSTNAME=host.docker.internal
 ENV USE_HOST_NETWORK=false
 ENV WORKSPACE_BASE=/opt/workspace_base
 ENV OPEN_DEVIN_BUILD_VERSION=$OPEN_DEVIN_BUILD_VERSION

opendevin/core/config.py

@@ -145,6 +145,7 @@ class SandboxConfig(metaclass=Singleton):
     """Configuration for the sandbox.
 
     Attributes:
+        api_hostname: The hostname for the EventStream Runtime API.
         container_image: The container image to use for the sandbox.
         user_id: The user ID for the sandbox.
         timeout: The timeout for the sandbox.
@@ -164,6 +165,7 @@ class SandboxConfig(metaclass=Singleton):
             Default is None for general purpose browsing. Check evaluation/miniwob and evaluation/webarena for examples.
     """
 
+    api_hostname: str = 'localhost'
     container_image: str = (
         'ubuntu:22.04'  # default to ubuntu:22.04 for eventstream runtime
     )

opendevin/runtime/client/client.py

@@ -128,14 +128,19 @@ def _init_user(self, username: str, user_id: int) -> None:
             raise RuntimeError(f'Failed to add sudoer: {output.stderr.decode()}')
         logger.debug(f'Added sudoer successfully. Output: [{output.stdout.decode()}]')
 
-        # Add user and change ownership of the initial working directory
+        # Add user and change ownership of the initial working directory if it doesn't exist
+        command = (
+            f'useradd -rm -d /home/{username} -s /bin/bash '
+            f'-g root -G sudo -u {user_id} {username}'
+        )
+
+        if not os.path.exists(self.initial_pwd):
+            command += f' && mkdir -p {self.initial_pwd}'
+            command += f' && chown -R {username}:root {self.initial_pwd}'
+            command += f' && chmod g+s {self.initial_pwd}'
+
         output = subprocess.run(
-            (
-                f'useradd -rm -d /home/{username} -s /bin/bash '
-                f'-g root -G sudo -u {user_id} {username} &&'
-                f'chown -R {username}:root {self.initial_pwd} && '
-                f'chmod g+s {self.initial_pwd}'
-            ),
+            command,
             shell=True,
             capture_output=True,
         )
@@ -381,11 +386,11 @@ async def write(self, action: FileWriteAction) -> Observation:
                     assert file_stat is not None
                     # restore the original file permissions if the file already exists
                     os.chmod(filepath, file_stat.st_mode)
-                    os.chown(filepath, file_stat.st_uid, ROOT_GID)
+                    os.chown(filepath, file_stat.st_uid, file_stat.st_gid)
                 else:
                     # set the new file permissions if the file is new
                     os.chmod(filepath, 0o644)
-                    os.chown(filepath, self.user_id, ROOT_GID)
+                    os.chown(filepath, self.user_id, self.user_id)
 
             except FileNotFoundError:
                 return ErrorObservation(f'File not found: {filepath}')

opendevin/runtime/client/runtime.py

@@ -22,7 +22,6 @@
 )
 from opendevin.events.action.action import Action
 from opendevin.events.observation import (
-    CmdOutputObservation,
     ErrorObservation,
     NullObservation,
     Observation,
@@ -54,7 +53,7 @@ def __init__(
             config, event_stream, sid, plugins
         )  # will initialize the event stream
         self._port = find_available_tcp_port()
-        self.api_url = f'http://localhost:{self._port}'
+        self.api_url = f'http://{self.config.sandbox.api_hostname}:{self._port}'
         self.session: Optional[aiohttp.ClientSession] = None
 
         self.instance_id = (
@@ -98,8 +97,6 @@ async def ainit(self, env_vars: dict[str, str] | None = None):
         )
         logger.info(f'Container initialized with env vars: {env_vars}')
 
-        await self._init_git_config()
-
     @staticmethod
     def _init_docker_client() -> docker.DockerClient:
         try:
@@ -183,16 +180,6 @@ async def _init_container(
             await self.close(close_client=False)
             raise e
 
-    async def _init_git_config(self):
-        action = CmdRunAction(
-            'git config --global user.name "opendevin" && '
-            'git config --global user.email "[email protected]"'
-        )
-        logger.info(f'Setting git config: {action}')
-        obs: Observation = await self.run_action(action)
-        assert isinstance(obs, CmdOutputObservation)
-        assert obs.exit_code == 0, f'Failed to set git config: {obs}'
-
     async def _ensure_session(self):
         await asyncio.sleep(1)
         if self.session is None or self.session.closed:
@@ -205,6 +192,20 @@ async def _ensure_session(self):
     )
     async def _wait_until_alive(self):
         logger.info('Reconnecting session')
+        container = self.docker_client.containers.get(self.container_name)
+        # print logs
+        _logs = container.logs(tail=10).decode('utf-8').split('\n')
+        # add indent
+        _logs = '\n'.join([f'    |{log}' for log in _logs])
+        logger.info(
+            '\n'
+            + '-' * 30
+            + 'Container logs (last 10 lines):'
+            + '-' * 30
+            + f'\n{_logs}'
+            + '\n'
+            + '-' * 90
+        )
         async with aiohttp.ClientSession() as session:
             async with session.get(f'{self.api_url}/alive') as response:
                 if response.status == 200:

tests/unit/test_runtime.py

@@ -1265,9 +1265,12 @@ async def test_keep_prompt(temp_dir):
 
 
 @pytest.mark.asyncio
-async def test_git_operation(temp_dir, box_class):
+async def test_git_operation(box_class):
+    # do not mount workspace, since workspace mount by tests will be owned by root
+    # while the user_id we get via os.getuid() is different from root
+    # which causes permission issues
     runtime = await _load_runtime(
-        temp_dir,
+        temp_dir=None,
         box_class=box_class,
         # Need to use non-root user to expose issues
         run_as_devin=True,