fix: dubious ownership when running git (#3282)

* switch default to eventstream runtime

* remove pull docker from makefile

* fix unittest

* fix file store path

* try deprecate server runtime

* remove persist sandbox

* move file utils

* remove server runtime related workflow

* remove unused method

* attempt to remove the reliance on filestore for BE

* fix async for list file

* fix list_files to post

* fix list files

* add suffix to directory

* make sure list file returns abs path;
make sure other backend endpoints accpets abs path

* remove server runtime test workflow

* set git config in runtime

* chown for workspace in client;
use INIT_COMMANDS to maintain all commands that need to be run before bash start;

* fix client issue;
add test case for git;

---------

Co-authored-by: Graham Neubig <neubig@gmail.com>

opendevin/runtime/client/client.py

@@ -56,6 +56,14 @@ class ActionRequest(BaseModel):
     action: dict
 
 
+ROOT_GID = 0
+INIT_COMMANDS = [
+    'git config --global user.name "opendevin"',
+    'git config --global user.email "opendevin@all-hands.dev"',
+    "alias git='git --no-pager'",
+]
+
+
 class RuntimeClient:
     """RuntimeClient is running inside docker sandbox.
     It is responsible for executing actions received from OpenDevin backend and producing observations.
@@ -73,6 +81,7 @@ class RuntimeClient:
         self.username = username
         self.user_id = user_id
         self.pwd = work_dir  # current PWD
+        self._initial_pwd = work_dir
         self._init_user(self.username, self.user_id)
         self._init_bash_shell(self.pwd, self.username)
         self.lock = asyncio.Lock()
@@ -104,6 +113,8 @@ class RuntimeClient:
             )
             logger.info(f'AgentSkills initialized: {obs}')
 
+        await self._init_bash_commands()
+
     def _init_user(self, username: str, user_id: int) -> None:
         """Create user if not exists."""
         # Skip root since it is already created
@@ -117,11 +128,13 @@ class RuntimeClient:
             raise RuntimeError(f'Failed to add sudoer: {output.stderr.decode()}')
         logger.debug(f'Added sudoer successfully. Output: [{output.stdout.decode()}]')
 
-        # Add user
+        # Add user and change ownership of the initial working directory
         output = subprocess.run(
             (
                 f'useradd -rm -d /home/{username} -s /bin/bash '
-                f'-g root -G sudo -u {user_id} {username}'
+                f'-g root -G sudo -u {user_id} {username} &&'
+                f'chown -R {username}:root {self.initial_pwd} && '
+                f'chmod g+s {self.initial_pwd}'
             ),
             shell=True,
             capture_output=True,
@@ -130,6 +143,7 @@ class RuntimeClient:
             raise RuntimeError(
                 f'Failed to create user {username}: {output.stderr.decode()}'
             )
+
         logger.debug(
             f'Added user {username} successfully. Output: [{output.stdout.decode()}]'
         )
@@ -156,6 +170,20 @@ class RuntimeClient:
             f'Bash initialized. Working directory: {work_dir}. Output: {self.shell.before}'
         )
 
+    async def _init_bash_commands(self):
+        logger.info(f'Initializing by running {len(INIT_COMMANDS)} bash commands...')
+        for command in INIT_COMMANDS:
+            action = CmdRunAction(command=command)
+            action.timeout = 300
+            logger.debug(f'Executing init command: {command}')
+            obs: CmdOutputObservation = await self.run(action)
+            logger.debug(
+                f'Init command outputs (exit code: {obs.exit_code}): {obs.content}'
+            )
+            assert obs.exit_code == 0
+
+        logger.info('Bash init commands completed')
+
     def _get_bash_prompt_and_update_pwd(self):
         ps1 = self.shell.after
 
@@ -353,11 +381,11 @@ class RuntimeClient:
                     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, file_stat.st_gid)
+                    os.chown(filepath, file_stat.st_uid, ROOT_GID)
                 else:
                     # set the new file permissions if the file is new
                     os.chmod(filepath, 0o644)
-                    os.chown(filepath, self.user_id, self.user_id)
+                    os.chown(filepath, self.user_id, ROOT_GID)
 
             except FileNotFoundError:
                 return ErrorObservation(f'File not found: {filepath}')

tests/unit/test_runtime.py

@@ -1262,3 +1262,81 @@ async def test_keep_prompt(temp_dir):
 
     await runtime.close()
     await asyncio.sleep(1)
+
+
+@pytest.mark.asyncio
+async def test_git_operation(temp_dir, box_class):
+    runtime = await _load_runtime(
+        temp_dir,
+        box_class=box_class,
+        # Need to use non-root user to expose issues
+        run_as_devin=True,
+    )
+
+    # this will happen if permission of runtime is not properly configured
+    # fatal: detected dubious ownership in repository at '/workspace'
+
+    # check the ownership of the current directory
+    action = CmdRunAction(command='ls -alh .')
+    logger.info(action, extra={'msg_type': 'ACTION'})
+    obs = await runtime.run_action(action)
+    logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+    assert isinstance(obs, CmdOutputObservation)
+    assert obs.exit_code == 0
+    # drwx--S--- 2 opendevin root   64 Aug  7 23:32 .
+    # drwxr-xr-x 1 root      root 4.0K Aug  7 23:33 ..
+    for line in obs.content.split('\r\n'):
+        if ' ..' in line:
+            # parent directory should be owned by root
+            assert 'root' in line
+            assert 'opendevin' not in line
+        elif ' .' in line:
+            # current directory should be owned by opendevin
+            # and its group should be root
+            assert 'opendevin' in line
+            assert 'root' in line
+
+    # make sure all git operations are allowed
+    action = CmdRunAction(command='git init')
+    logger.info(action, extra={'msg_type': 'ACTION'})
+    obs = await runtime.run_action(action)
+    logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+    assert isinstance(obs, CmdOutputObservation)
+    assert obs.exit_code == 0
+
+    # create a file
+    action = CmdRunAction(command='echo "hello" > test_file.txt')
+    logger.info(action, extra={'msg_type': 'ACTION'})
+    obs = await runtime.run_action(action)
+    logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+    assert isinstance(obs, CmdOutputObservation)
+    assert obs.exit_code == 0
+
+    # git add
+    action = CmdRunAction(command='git add test_file.txt')
+    logger.info(action, extra={'msg_type': 'ACTION'})
+    obs = await runtime.run_action(action)
+    logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+    assert isinstance(obs, CmdOutputObservation)
+    assert obs.exit_code == 0
+
+    # git diff
+    action = CmdRunAction(command='git diff')
+    logger.info(action, extra={'msg_type': 'ACTION'})
+    obs = await runtime.run_action(action)
+    logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+    assert isinstance(obs, CmdOutputObservation)
+    assert obs.exit_code == 0
+
+    # git commit
+    action = CmdRunAction(command='git commit -m "test commit"')
+    logger.info(action, extra={'msg_type': 'ACTION'})
+    obs = await runtime.run_action(action)
+    logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+    assert isinstance(obs, CmdOutputObservation)
+    assert obs.exit_code == 0
+
+    await runtime.close()
+
+    await runtime.close()
+    await asyncio.sleep(1)