Avoid shell injection problems by using subprocess.run()

CHANGELOG.md

@@ -1,5 +1,9 @@
 # CHANGELOG
 
+## 3.0.0
+
+- The anonymizer option is back
+
 ## 2.1.5
 
 ## 2.1.4 - 2019-04-04

dcm2bids/dcm2bids.py

@@ -7,13 +7,14 @@
 import os
 import platform
 import sys
+import subprocess
 from datetime import datetime
 from glob import glob
 from .dcm2niix import Dcm2niix
 from .logger import setup_logging
 from .sidecar import Sidecar, SidecarPairing
 from .structure import Participant
-from .utils import DEFAULT, load_json, save_json, run_shell_command, splitext_
+from .utils import DEFAULT, load_json, save_json, splitext_
 from .version import __version__, check_latest, dcm2niix_version
 
 
@@ -166,17 +167,17 @@ def move(self, acquisition):
 
             # it's an anat nifti file and the user using a deface script
             if (
-                self.config.get("defaceTpl")
+                self.config.get("deface")
                 and acquisition.dataType == "anat"
                 and ".nii" in ext
             ):
                 try:
                     os.remove(dstFile)
                 except FileNotFoundError:
                     pass
-                defaceTpl = self.config.get("defaceTpl")
-                cmd = defaceTpl.format(srcFile=srcFile, dstFile=dstFile)
-                run_shell_command(cmd)
+                cmd = ["pydeface", "--outfile", dstFile, srcFile]
+                self.logger.info("Running %s" % (shlex.join(cmd),))
+                subprocess.run(cmd, check=True)
 
             # use
             elif ext == ".json":
@@ -260,7 +261,7 @@ def get_arguments():
         action="store_true",
         help="""
         This option no longer exists from the script in this release.
-        See:https://github.com/unfmontreal/Dcm2Bids/blob/master/README.md#defaceTpl""",
+        See:https://github.com/unfmontreal/Dcm2Bids/blob/master/README.md#deface""",
     )
 
     args = parser.parse_args()
@@ -276,12 +277,10 @@ def main():
             """
         The anonymizer option no longer exists from the script in this release
         It is still possible to deface the anatomical nifti images
-        Please add "defaceTpl" key in the congifuration file
+        Please add "deface" key in the configuration file
 
         For example, if you use the last version of pydeface, add:
-        "defaceTpl": "pydeface --outfile {dstFile} {srcFile}"
-        It is a template string and dcm2bids will replace {srcFile} and {dstFile}
-        by the source file (input) and the destination file (output)
+        "deface": true
         """
         )
         return 1

dcm2bids/dcm2niix.py

@@ -4,9 +4,11 @@
 
 import logging
 import os
+import shlex
 import shutil
+import subprocess
 from glob import glob
-from .utils import DEFAULT, run_shell_command
+from .utils import DEFAULT
 
 
 class Dcm2niix(object):
@@ -95,14 +97,6 @@ def execute(self):
         """ Execute dcm2niix for each directory in dicomDirs
         """
         for dicomDir in self.dicomDirs:
-            commandTpl = "dcm2niix {} -o {} {}"
-            cmd = commandTpl.format(self.options, self.outputDir, dicomDir)
-            output = run_shell_command(cmd)
-
-            try:
-                output = output.decode()
-            except:
-                pass
-
-            self.logger.debug("\n%s", output)
-            self.logger.info("Check log file for dcm2niix output")
+            cmd = ['dcm2niix', *shlex.split(self.options), '-o', self.outputDir, dicomDir]
+            self.logger.info("Running %s" % (shlex.join(cmd),))
+            subprocess.run(cmd, check=True)

dcm2bids/utils.py

@@ -7,7 +7,6 @@
 import os
 import shlex
 from collections import OrderedDict
-from subprocess import check_output
 
 
 class DEFAULT(object):
@@ -23,7 +22,7 @@ class DEFAULT(object):
     session = cliSession  # also Participant object
     clobber = False
     forceDcm2niix = False
-    defaceTpl = None
+    deface = False
     logLevel = "WARNING"
 
     # dcm2niix.py
@@ -99,14 +98,3 @@ def splitext_(path, extensions=None):
         if path.endswith(ext):
             return path[: -len(ext)], path[-len(ext) :]
     return os.path.splitext(path)
-
-
-def run_shell_command(commandLine):
-    """ Wrapper of subprocess.check_output
-
-    Returns:
-        Run command with arguments and return its output
-    """
-    logger = logging.getLogger(__name__)
-    logger.info("Running %s", commandLine)
-    return check_output(shlex.split(commandLine))

dcm2bids/version.py

@@ -3,7 +3,7 @@
 """This module take care of the versioning"""
 
 # dcm2bids version
-__version__ = "2.1.5"
+__version__ = "3.0.0"
 
 
 import logging
@@ -14,8 +14,6 @@
 from shutil import which
 
 
-REQUIRED_MODULE_METADATA = (("future", {"min_version": "0.17.1"}),)
-
 logger = logging.getLogger(__name__)
 
 
@@ -64,7 +62,7 @@ def check_github_latest(githubRepo):
     """
     url = "https://github.com/{}/releases/latest".format(githubRepo)
     try:
-        output = check_output(shlex.split("curl --silent " + url))
+        output = check_output(["curl", "--silent", url])
     except:
         logger.debug(
             "Checking latest version of %s was not possible", githubRepo,
@@ -153,7 +151,7 @@ def dcm2niix_version():
         return
 
     try:
-        output = check_output(shlex.split("dcm2niix"))
+        output = check_output(["dcm2niix"])
     except:
         logger.error("Running: dcm2niix", exc_info=True)
         return

docs/4-advance.md

@@ -5,7 +5,7 @@ These optional configurations could be insert in the configuration file at the s
 ```
 {
     "searchMethod": "fnmatch",
-    "defaceTpl": "pydeface --outfile {dstFile} {srcFile}",
+    "deface": true,
     "description": [
         ...
     ]
@@ -18,17 +18,11 @@ default: `"searchMethod": "fnmatch"`
 
 fnmatch is the behaviour (See criteria) by default and the fall back if this option is set incorrectly. `re` is the other choice if you want more flexibility to match criteria.
 
-## defaceTpl
+## deface
 
-default: `"defaceTpl": None`
+default: `"deface": False`
 
-The anonymizer option no longer exists from `v2.0.0`. It is still possible to deface the anatomical nifti images.
-
-For example, if you use the last version of pydeface, add:
-
-`"defaceTpl": "pydeface --outfile {dstFile} {srcFile}"`
-
-It is a template string and dcm2bids will replace {srcFile} and {dstFile} by the source file (input) and the destination file (output).
+To anonymize images with `pydeface`, set this to `True`.
 
 ## dcm2niixOptions
 

example/config.json

@@ -1,6 +1,6 @@
 {
     "searchMethod": "fnmatch",
-    "defaceTpl": "pydeface --outfile {dstFile} {srcFile}",
+    "deface": true,
     "descriptions": [
         {
             "dataType": "anat",

setup.py

@@ -17,12 +17,10 @@ def load_version():
     return global_dict
 
 
-def install_requires():
-    """Get list of required modules"""
-    required = []
-    for module, meta in _VERSION["REQUIRED_MODULE_METADATA"]:
-        required.append("{}>={}".format(module, meta["min_version"]))
-    return required
+install_requires = [
+ "future>=0.17.1",
+ "pydeface",
+]
 
 
 _VERSION = load_version()
@@ -71,7 +69,7 @@ def install_requires():
         packages=find_packages(),
         entry_points=ENTRY_POINTS,
         python_requires=">=3.5",
-        install_requires=install_requires(),
+        install_requires=install_requires,
         package_data={"": ["README.md", "LICENSE.txt"]},
         author=AUTHOR,
         author_email=AUTHOR_EMAIL,