Check entryname (#1324)

* Check for valid values of entryname per 1.2 spec
* Formalize the 1.2 feature of mounting to arbitrary locations

Allow internal control of pathmapping by setting 'dirname'.

For containers only.

Also clean up paths with '..', check for invalid paths, and ensure
that absolute paths can only be used when DockerRequirement is in
requirements.

Author: tetron

cwltool/command_line_tool.py

@@ -79,6 +79,7 @@
     visit_class,
     windows_default_container_id,
 )
+from .update import ORDERED_VERSIONS
 
 if TYPE_CHECKING:
     from .provenance_profile import ProvenanceProfile  # pylint: disable=unused-import
@@ -215,6 +216,9 @@ def revmap_file(
         else:
             return f
 
+    if "dirname" in f:
+        del f["dirname"]
+
     if "path" in f:
         path = cast(str, f["path"])
         uripath = file_uri(path)
@@ -444,6 +448,10 @@ def make_path_mapper(
     def updatePathmap(
         self, outdir: str, pathmap: PathMapper, fn: CWLObjectType
     ) -> None:
+        if not isinstance(fn, MutableMapping):
+            raise WorkflowException(
+                "Expected File or Directory object, was %s" % type(fn)
+            )
         basename = cast(str, fn["basename"])
         if "location" in fn:
             location = cast(str, fn["location"])
@@ -607,20 +615,71 @@ def _initialworkdir(self, j: JobBase, builder: Builder) -> None:
                     "Entry at index %s of listing is not a Dirent, File or Directory object, was %s"
                     % (i, t2)
                 )
+            if "basename" not in t3:
+                continue
+            basename = os.path.normpath(cast(str, t3["basename"]))
+            t3["basename"] = basename
+            if basename.startswith("../"):
+                raise SourceLine(
+                    initialWorkdir, "listing", WorkflowException
+                ).makeError(
+                    "Name '%s' at index %s of listing is invalid, cannot start with '../'"
+                    % (basename, i)
+                )
+            if basename.startswith("/"):
+                # only if DockerRequirement in requirements
+                cwl_version = self.metadata.get(
+                    "http://commonwl.org/cwltool#original_cwlVersion", None
+                )
+                if isinstance(cwl_version, str) and ORDERED_VERSIONS.index(
+                    cwl_version
+                ) < ORDERED_VERSIONS.index("v1.2.0-dev4"):
+                    raise SourceLine(
+                        initialWorkdir, "listing", WorkflowException
+                    ).makeError(
+                        "Name '%s' at index %s of listing is invalid, paths starting with '/' only permitted in CWL 1.2 and later"
+                        % (basename, i)
+                    )
 
-        normalizeFilesDirs(ls)
+                req, is_req = self.get_requirement("DockerRequirement")
+                if is_req is not True:
+                    raise SourceLine(
+                        initialWorkdir, "listing", WorkflowException
+                    ).makeError(
+                        "Name '%s' at index %s of listing is invalid, name can only start with '/' when DockerRequirement is in 'requirements'"
+                        % (basename, i)
+                    )
 
-        j.generatefiles["listing"] = ls
-        for entry in ls:
-            self.updatePathmap(
-                builder.outdir, cast(PathMapper, builder.pathmapper), entry
-            )
+        with SourceLine(initialWorkdir, "listing", WorkflowException):
+            j.generatefiles["listing"] = ls
+            for entry in ls:
+                if "basename" in entry:
+                    basename = cast(str, entry["basename"])
+                    entry["dirname"] = os.path.join(
+                        builder.outdir, os.path.dirname(basename)
+                    )
+                    entry["basename"] = os.path.basename(basename)
+                normalizeFilesDirs(entry)
+                self.updatePathmap(
+                    cast(Optional[str], entry.get("dirname")) or builder.outdir,
+                    cast(PathMapper, builder.pathmapper),
+                    entry,
+                )
+                if "listing" in entry:
 
-        visit_class(
-            [builder.files, builder.bindings],
-            ("File", "Directory"),
-            partial(check_adjust, builder),
-        )
+                    def remove_dirname(d: CWLObjectType) -> None:
+                        if "dirname" in d:
+                            del d["dirname"]
+
+                    visit_class(
+                        entry["listing"], ("File", "Directory"), remove_dirname,
+                    )
+
+            visit_class(
+                [builder.files, builder.bindings],
+                ("File", "Directory"),
+                partial(check_adjust, builder),
+            )
 
     def job(
         self,

cwltool/pathmapper.py

@@ -116,8 +116,9 @@ def visit(
         copy: bool = False,
         staged: bool = False,
     ) -> None:
+        stagedir = cast(Optional[str], obj.get("dirname")) or stagedir
         tgt = convert_pathsep_to_unix(
-            os.path.join(stagedir, cast(str, obj["basename"]))
+            os.path.join(stagedir, cast(str, obj["basename"]),)
         )
         if obj["location"] in self._pathmap:
             return

cwltool/process.py

@@ -1235,7 +1235,10 @@ def scandeps(
                     if nestdirs:
                         deps = nestdir(base, deps)
                     r.append(deps)
-            elif doc.get("class") in ("File", "Directory") and k in ("listing", "secondaryFiles"):
+            elif doc.get("class") in ("File", "Directory") and k in (
+                "listing",
+                "secondaryFiles",
+            ):
                 # should be handled earlier.
                 pass
             else:

tests/test_iwdr.py

@@ -63,6 +63,7 @@ def test_iwdr_permutations() -> None:
                                                     [
                                                         "--outdir",
                                                         outdir,
+                                                        "--enable-dev",
                                                         get_data(
                                                             "tests/wf/iwdr_permutations.cwl"
                                                         ),
@@ -109,6 +110,7 @@ def test_iwdr_permutations_inplace() -> None:
                                                         "--outdir",
                                                         outdir,
                                                         "--enable-ext",
+                                                        "--enable-dev",
                                                         "--overrides",
                                                         get_data(
                                                             "tests/wf/iwdr_permutations_inplace.yml"
@@ -155,6 +157,7 @@ def test_iwdr_permutations_singularity() -> None:
                                                 [
                                                     "--outdir",
                                                     outdir,
+                                                    "--enable-dev",
                                                     "--singularity",
                                                     get_data(
                                                         "tests/wf/iwdr_permutations.cwl"
@@ -199,6 +202,7 @@ def test_iwdr_permutations_singularity_inplace() -> None:
                                                     outdir,
                                                     "--singularity",
                                                     "--enable-ext",
+                                                    "--enable-dev",
                                                     "--overrides",
                                                     get_data(
                                                         "tests/wf/iwdr_permutations_inplace.yml"

tests/wf/iwdr_permutations.cwl

@@ -1,6 +1,6 @@
 #!/usr/bin/env cwl-runner
 class: CommandLineTool
-cwlVersion: v1.0
+cwlVersion: v1.2.0-dev4
 requirements:
   DockerRequirement:
     dockerPull: debian
@@ -74,4 +74,3 @@ arguments:
     echo "b" > /my_path/third_writable_file
     touch fifth_writable_directory/c
     touch /my_path/seventh_writable_directory/d
-