Repository: codalab/codalab-worksheets

Issue:
Run cl run --interactive as a non-root user
**Describe the bug**
Run `cl run --interactive` as a non-root user to match the same behavior as CodaLab worker running jobs.


Discussion:
teetone: > @teetone This all looks good -- do you think it would be possible to add an automated test for `cl run --interactive` in the future (probably could make as a separate issue / PR)? This code is pretty complex here, so I'd like to add a test so we can ensure nothing regresses. Maybe we could use something like https://ptyprocess.readthedocs.io/en/latest/ to make a simple test.

I think it's a bit difficult to test end-to-end because we expect user input in the container and most of the complexity is in how we start the interactive session container i.e. `get_docker_run_command`. I'm open to adding tests in a separate PR though.

Patch:
diff --git a/codalab/lib/interactive_session.py b/codalab/lib/interactive_session.py
index 73b252be..2164e2c9 100644
--- a/codalab/lib/interactive_session.py
+++ b/codalab/lib/interactive_session.py
@@ -6,6 +6,7 @@ import docker
 import os
 import shutil
 import sys
+from typing import Dict, List
 
 
 class InteractiveSession:
@@ -23,7 +24,7 @@ class InteractiveSession:
         3. Mount the dependencies as read-only.
         4. Set working directory to some arbitrary path: `/<session uuid>`
         5. The user will interact with this container and try out different commands. Once satisfied the user will
-           exit the bash session. All the commands the user tried out will be stored at path `/root/.bash_history`
+           exit the bash session. All the commands the user tried out will be stored at path `/usr/sbin/.bash_history`
            in the container.
         6. Copy `.bash_history` out to the host machine.
         7. Open an editor and allow the user to select and edit commands for the official run.
@@ -31,7 +32,7 @@ class InteractiveSession:
         9. Stop and remove the interactive session container.
     """
 
-    _BASH_HISTORY_CONTAINER_PATH = "/root/.bash_history"
+    _BASH_HISTORY_CONTAINER_PATH = "/usr/sbin/.bash_history"
     _CHOOSE_COMMAND_INSTRUCTIONS = (
         "\n\n#\n"
         "# Choose the commands to use for cl run:\n"
@@ -74,15 +75,15 @@ class InteractiveSession:
 
         self._docker_client = docker.from_env(timeout=InteractiveSession._MAX_SESSION_TIMEOUT)
         self._session_uuid = generate_uuid()
+        self._host_bundle_path: str = os.path.join(
+            self._manager.codalab_home, 'local_bundles', self._session_uuid
+        )
 
         self._verbose = verbose
         self._stdout = stdout
         self._stderr = stderr
 
     def start(self):
-        self._host_bundle_path = os.path.join(
-            self._manager.codalab_home, 'local_bundles', self._session_uuid
-        )
         os.makedirs(self._host_bundle_path)
 
         # Create a blank file which will be used as the bash history file that will later be
@@ -123,7 +124,7 @@ class InteractiveSession:
             return os.path.sep + os.path.join(self._session_uuid, sub_path)
 
         # Use a dict to keep track of volumes to mount. The key is the path on Docker and the value is the local path.
-        volumes = {}
+        volumes: Dict[str, str] = {}
         for key, bundle_target in self._dependencies:
             dependency_local_path = os.path.realpath(
                 os.path.join(
@@ -142,30 +143,34 @@ class InteractiveSession:
             else:
                 volumes[get_docker_path(key)] = dependency_local_path
 
-        name = self._get_container_name()
-        # Start a container as a root user. Root (id = 0) is the default user within a container.
-        command = [
+        name: str = self._get_container_name()
+        container_working_directory: str = f'{os.path.sep}{self._session_uuid}'
+
+        # Start a container as a non-root user
+        command: List[str] = [
             'docker run',
             '-it',
             f'--name {name}',
-            f'-w {os.path.sep}{self._session_uuid}',
-            # TODO: figure out a way to run as a non-root user and be able to save the bash_history on any machine
-            # '-u 1',
+            f'-w {container_working_directory}',
+            f'-e HOME={container_working_directory}',
+            f'-e HISTFILE={InteractiveSession._BASH_HISTORY_CONTAINER_PATH}',
+            '-e PROMPT_COMMAND="history -a"',
+            '-u $(id -u):$(id -g)',
         ]
         command.extend(
             [
                 # Example: -v local_path/some_folder:/0x707e903500e54bcf9b072ac7e3f5ed36_dependencies/foo:ro
-                '-v {}:{}:ro'.format(local_path, docker_path)
+                f'-v {local_path}:{docker_path}:ro'
                 for docker_path, local_path in volumes.items()
             ]
         )
-        command.append(
-            '-v {}:{}:rw'.format(
-                self._host_bash_history_path, InteractiveSession._BASH_HISTORY_CONTAINER_PATH
-            )
+        command.extend(
+            [
+                f'-v {self._host_bash_history_path}:{InteractiveSession._BASH_HISTORY_CONTAINER_PATH}:rw',
+                f'-v {self._host_bundle_path}:{container_working_directory}:rw',
+            ]
         )
         command.append(self._docker_image)
-        command.append('bash')
         return ' '.join(command)
 
     def cleanup(self):
@@ -175,6 +180,7 @@ class InteractiveSession:
         self._container.stop()
         self._container.remove()
         shutil.rmtree(self._host_bundle_path, ignore_errors=True)
+
         if self._verbose:
             print('Done.\n', file=self._stdout)
 


Test patch:
diff --git a/tests/unit/lib/interactive_session_test.py b/tests/unit/lib/interactive_session_test.py
index f5c5ab6d..6fb78c06 100644
--- a/tests/unit/lib/interactive_session_test.py
+++ b/tests/unit/lib/interactive_session_test.py
@@ -14,9 +14,10 @@ class InteractiveSessionTest(unittest.TestCase):
         )
         session._host_bash_history_path = ".bash_history"
         expected_regex = (
-            'docker run -it --name interactive-session-0x[a-z0-9]{32} -w \/0x[a-z0-9]{32} -v '
-            '[\s\S]{0,100}local\/path1:\/0x[a-z0-9]{32}\/key:ro -v [\s\S]{0,100}local\/path2:\/0x[a-z0-9]{32}\/key2:ro '
-            '-v \.bash_history:\/root\/.bash_history:rw some-docker-image bash'
+            'docker run -it --name interactive-session-0x[a-z0-9]{32} -w \/0x[a-z0-9]{32} '
+            '-e HOME=\/0x[a-z0-9]{32} -e HISTFILE=\/usr\/sbin\/\.bash_history -e PROMPT_COMMAND="history -a" -u \$\(id -u\):\$\(id -g\) '
+            '-v [\s\S]{0,100}local\/path1:\/0x[a-z0-9]{32}\/key:ro -v [\s\S]{0,100}local\/path2:\/0x[a-z0-9]{32}\/key2:ro '
+            '-v \.bash_history:\/usr\/sbin\/\.bash_history:rw -v [\s\S]{0,100}\/0x[a-z0-9]{32}:\/0x[a-z0-9]{32}:rw some-docker-image'
         )
         self.assertTrue(re.match(expected_regex, session.get_docker_run_command()))
 
@@ -31,9 +32,10 @@ class InteractiveSessionTest(unittest.TestCase):
         )
         session._host_bash_history_path = ".bash_history"
         expected_regex = (
-            'docker run -it --name interactive-session-0x[a-z0-9]{32} -w \/0x[a-z0-9]{32} -v '
-            '[\s\S]{0,100}local\/path1/sub/path1:\/0x[a-z0-9]{32}\/key:ro -v [\s\S]{0,100}local\/path2/sub/path2'
-            ':\/0x[a-z0-9]{32}\/key2:ro -v \.bash_history:\/root\/.bash_history:rw some-docker-image bash'
+            'docker run -it --name interactive-session-0x[a-z0-9]{32} -w \/0x[a-z0-9]{32} '
+            '-e HOME=\/0x[a-z0-9]{32} -e HISTFILE=\/usr\/sbin\/\.bash_history -e PROMPT_COMMAND="history -a" -u \$\(id -u\):\$\(id -g\) '
+            '-v [\s\S]{0,100}local\/path1/sub/path1:\/0x[a-z0-9]{32}\/key:ro -v [\s\S]{0,100}local\/path2/sub/path2:\/0x[a-z0-9]{32}\/key2:ro '
+            '-v \.bash_history:\/usr\/sbin\/\.bash_history:rw -v [\s\S]{0,100}\/0x[a-z0-9]{32}:\/0x[a-z0-9]{32}:rw some-docker-image'
         )
         self.assertTrue(re.match(expected_regex, session.get_docker_run_command()))