Restrict output to specific dirs

bcl2fastq/handlers/bcl2fastq_handlers.py

@@ -119,14 +119,7 @@ def create_config_from_request(self, runfolder, request_body):
             bcl2fastq_version = request_data["bcl2fastq_version"]
 
         if "output" in request_data:
-            if self.config['allow_arbitrary_output_folder']:
-                output = request_data["output"]
-            else:
-                raise ArteriaUsageException("A output folder was specified by the request, but the configuration "
-                                            "does not allow arbitrary output folder to be specified.")
-
-        if os.path.abspath(output) == os.path.abspath(runfolder_input):
-            raise ArteriaUsageException("The specified output path is the same as the input path. This is not allowed!")
+            output = request_data["output"]
 
         if "samplesheet" in request_data:
             samplesheet = request_data["samplesheet"]

bcl2fastq/lib/bcl2fastq_utils.py

@@ -48,6 +48,8 @@ def __init__(self,
         :param nbr_of_cores: number of cores to run bcl2fastq with
         """
 
+        self.general_config = general_config
+
         self.runfolder_input = runfolder_input
         self.base_calls_input = runfolder_input + "/Data/Intensities/BaseCalls"
 
@@ -297,12 +299,29 @@ def construct_command(self):
         """
         raise NotImplementedError("Subclasses should implement this!")
 
+    def validate_output(self):
+
+        def _parent_dir(d):
+            return os.path.abspath(os.path.join(d, os.path.pardir))
+
+        abs_path_of_allowed_dirs = map(os.path.abspath, self.config.general_config['allowed_output_folders'])
+        is_located_in_parent_dir = _parent_dir(self.config.output) in abs_path_of_allowed_dirs
+
+        if is_located_in_parent_dir:
+            True
+        else:
+            error_string = "Invalid output directory {} was specified." \
+                           " Allowed dirs were: {}".format(self.config.output,
+                                                           self.config.general_config['allowed_output_folders'])
+            log.error(error_string)
+            raise ArteriaUsageException(error_string)
+
     def delete_output(self):
         """
-        Delete the output directory if it exists
+        Delete the output directory if it exists and  the output path is valid
         :return: None
         """
-        if os.path.isdir(self.config.output):
+        if self.validate_output():
             log.info("Found a directory at output path {}, will remove it.".format(self.config.output))
             shutil.rmtree(self.config.output)
 

config/app.config

@@ -35,9 +35,8 @@ default_output_path: /vagrant/runfolder_output/
 
 bcl2fastq_logs_path: bcl2fastq_logs
 
-# Setting this to True will allow the bcl2fastq service direct
-# the output of bcl2fastq anywhere on the filesystem (restricted
-# by normal permissions). If this is set to False (as per default)
-# it will only write output to the specified `default_output_path`
-allow_arbitrary_output_folder: False
+# Only folders and child folder of the directories listed here will be valid as output
+# directories.
+allowed_output_folders:
+    -  /vagrant/runfolder_output/
 

tests/test_bcl2fastq_handlers.py

@@ -32,9 +32,8 @@ def start_api_call_nbr(self):
                             8: "y*,7i,n*,y*",
                             }
 
-    DUMMY_RUNNER_CONF = DummyRunnerConfig(output='/foo/bar')
-
     dummy_config = DummyConfig()
+    DUMMY_RUNNER_CONF = DummyRunnerConfig(output='/foo/bar/runfolder', general_config = dummy_config)
 
     def get_app(self):
         return Application(routes(config=self.dummy_config))
@@ -94,22 +93,32 @@ def test_start_with_empty_body(self):
 
     def test_start_with_disallowed_output_specified(self):
 
-        self.dummy_config.DUMMY_CONFIG['allow_arbitrary_output_folder'] = False
+        # TODO Please note that this test is not very good, since the
+        #      what we ideally want to test is the output specifed by the request,
+        #      and that that gets rejected, however since the create_bcl2fastq_runner command
+        #      is mocked away here and that is what returns the invalid output the tests is
+        #      a bit misleading. In the future we probably want to refactor this.
+        #      / JD 20170117
+        runner_conf_with_invalid_output = DummyRunnerConfig(output='/not/foo/bar/runfolder',
+                                                            general_config = self.dummy_config)
+        with mock.patch.object(os.path, 'isdir', return_value=True), \
+             mock.patch.object(shutil, 'rmtree', return_value=None), \
+             mock.patch.object(Bcl2FastqConfig, 'get_bcl2fastq_version_from_run_parameters', return_value="2.15.2"), \
+             mock.patch.object(BCL2FastqRunnerFactory, "create_bcl2fastq_runner",
+                               return_value=FakeRunner("2.15.2", runner_conf_with_invalid_output)), \
+             mock.patch.object(BCL2FastqRunner, 'symlink_output_to_unaligned', return_value=None):
 
-        with mock.patch.object(os.path, 'isdir', return_value=True):
-                body = {'output': '/foo/bar'}
+                # This output dir is not allowed by the config
+                body = {'output': '/not/foo/bar'}
                 response = self.fetch(self.API_BASE + "/start/150415_D00457_0091_AC6281ANXX",
                                       method="POST",
                                       body=json_encode(body))
+
                 self.assertEqual(response.code, 500)
-                self.assertEqual(response.reason, "A output folder was specified by the request, "
-                                                  "but the configuration does "
-                                                  "not allow arbitrary output folder to be specified.")
+                self.assertEqual(response.reason, "Invalid output directory /not/foo/bar/runfolder was specified."
+                                                  " Allowed dirs were: ['/foo/bar']")
 
     def test_start_with_allowed_output_specified(self):
-
-        self.dummy_config.DUMMY_CONFIG['allow_arbitrary_output_folder'] = True
-
         with mock.patch.object(os.path, 'isdir', return_value=True), \
             mock.patch.object(shutil, 'rmtree', return_value=None), \
             mock.patch.object(Bcl2FastqConfig, 'get_bcl2fastq_version_from_run_parameters', return_value="2.15.2"), \

tests/test_utils.py

@@ -54,14 +54,15 @@ class DummyConfig:
                           "HiSeq 2000": {"bcl2fastq_version": "1.8.4"},
                           "NextSeq 500": {"bcl2fastq_version": "1.8.4"}},
                      "bcl2fastq_logs_path": "/tmp/",
-                     "allow_arbitrary_output_folder": False}
+                     "allowed_output_folders": ['/foo/bar']}
 
     def __getitem__(self, key):
         return self.DUMMY_CONFIG[key]
 
 class DummyRunnerConfig(Bcl2FastqConfig):
-    def __init__(self, output):
+    def __init__(self, output, general_config):
         self.output = output
+        self.general_config = general_config
 
 
 class FakeRunner(BCL2FastqRunner):