Skip to content

Commit eba8091

Browse files
tetrontetron
authored
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.
1 parent cbc33f3 commit eba8091

File tree

5 files changed

+81
-15
lines changed

5 files changed

+81
-15
lines changed

‎cwltool/command_line_tool.py‎

Lines changed: 70 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@
7979
visit_class,
8080
windows_default_container_id,
8181
)
82+
from .update import ORDERED_VERSIONS
8283

8384
if TYPE_CHECKING:
8485
from .provenance_profile import ProvenanceProfile # pylint: disable=unused-import
@@ -215,6 +216,9 @@ def revmap_file(
215216
else:
216217
return f
217218

219+
if "dirname" in f:
220+
del f["dirname"]
221+
218222
if "path" in f:
219223
path = cast(str, f["path"])
220224
uripath = file_uri(path)
@@ -444,6 +448,10 @@ def make_path_mapper(
444448
def updatePathmap(
445449
self, outdir: str, pathmap: PathMapper, fn: CWLObjectType
446450
) -> None:
451+
if not isinstance(fn, MutableMapping):
452+
raise WorkflowException(
453+
"Expected File or Directory object, was %s" % type(fn)
454+
)
447455
basename = cast(str, fn["basename"])
448456
if "location" in fn:
449457
location = cast(str, fn["location"])
@@ -607,20 +615,71 @@ def _initialworkdir(self, j: JobBase, builder: Builder) -> None:
607615
"Entry at index %s of listing is not a Dirent, File or Directory object, was %s"
608616
% (i, t2)
609617
)
618+
if "basename" not in t3:
619+
continue
620+
basename = os.path.normpath(cast(str, t3["basename"]))
621+
t3["basename"] = basename
622+
if basename.startswith("../"):
623+
raise SourceLine(
624+
initialWorkdir, "listing", WorkflowException
625+
).makeError(
626+
"Name '%s' at index %s of listing is invalid, cannot start with '../'"
627+
% (basename, i)
628+
)
629+
if basename.startswith("/"):
630+
# only if DockerRequirement in requirements
631+
cwl_version = self.metadata.get(
632+
"http://commonwl.org/cwltool#original_cwlVersion", None
633+
)
634+
if isinstance(cwl_version, str) and ORDERED_VERSIONS.index(
635+
cwl_version
636+
) < ORDERED_VERSIONS.index("v1.2.0-dev4"):
637+
raise SourceLine(
638+
initialWorkdir, "listing", WorkflowException
639+
).makeError(
640+
"Name '%s' at index %s of listing is invalid, paths starting with '/' only permitted in CWL 1.2 and later"
641+
% (basename, i)
642+
)
610643

611-
normalizeFilesDirs(ls)
644+
req, is_req = self.get_requirement("DockerRequirement")
645+
if is_req is not True:
646+
raise SourceLine(
647+
initialWorkdir, "listing", WorkflowException
648+
).makeError(
649+
"Name '%s' at index %s of listing is invalid, name can only start with '/' when DockerRequirement is in 'requirements'"
650+
% (basename, i)
651+
)
612652

613-
j.generatefiles["listing"] = ls
614-
for entry in ls:
615-
self.updatePathmap(
616-
builder.outdir, cast(PathMapper, builder.pathmapper), entry
617-
)
653+
with SourceLine(initialWorkdir, "listing", WorkflowException):
654+
j.generatefiles["listing"] = ls
655+
for entry in ls:
656+
if "basename" in entry:
657+
basename = cast(str, entry["basename"])
658+
entry["dirname"] = os.path.join(
659+
builder.outdir, os.path.dirname(basename)
660+
)
661+
entry["basename"] = os.path.basename(basename)
662+
normalizeFilesDirs(entry)
663+
self.updatePathmap(
664+
cast(Optional[str], entry.get("dirname")) or builder.outdir,
665+
cast(PathMapper, builder.pathmapper),
666+
entry,
667+
)
668+
if "listing" in entry:
618669

619-
visit_class(
620-
[builder.files, builder.bindings],
621-
("File", "Directory"),
622-
partial(check_adjust, builder),
623-
)
670+
def remove_dirname(d: CWLObjectType) -> None:
671+
if "dirname" in d:
672+
del d["dirname"]
673+
674+
visit_class(
675+
entry["listing"], ("File", "Directory"), remove_dirname,
676+
)
677+
678+
visit_class(
679+
[builder.files, builder.bindings],
680+
("File", "Directory"),
681+
partial(check_adjust, builder),
682+
)
624683

625684
def job(
626685
self,

‎cwltool/pathmapper.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,9 @@ def visit(
116116
copy: bool = False,
117117
staged: bool = False,
118118
) -> None:
119+
stagedir = cast(Optional[str], obj.get("dirname")) or stagedir
119120
tgt = convert_pathsep_to_unix(
120-
os.path.join(stagedir, cast(str, obj["basename"]))
121+
os.path.join(stagedir, cast(str, obj["basename"]),)
121122
)
122123
if obj["location"] in self._pathmap:
123124
return

‎cwltool/process.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1235,7 +1235,10 @@ def scandeps(
12351235
if nestdirs:
12361236
deps = nestdir(base, deps)
12371237
r.append(deps)
1238-
elif doc.get("class") in ("File", "Directory") and k in ("listing", "secondaryFiles"):
1238+
elif doc.get("class") in ("File", "Directory") and k in (
1239+
"listing",
1240+
"secondaryFiles",
1241+
):
12391242
# should be handled earlier.
12401243
pass
12411244
else:

‎tests/test_iwdr.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ def test_iwdr_permutations() -> None:
6363
[
6464
"--outdir",
6565
outdir,
66+
"--enable-dev",
6667
get_data(
6768
"tests/wf/iwdr_permutations.cwl"
6869
),
@@ -109,6 +110,7 @@ def test_iwdr_permutations_inplace() -> None:
109110
"--outdir",
110111
outdir,
111112
"--enable-ext",
113+
"--enable-dev",
112114
"--overrides",
113115
get_data(
114116
"tests/wf/iwdr_permutations_inplace.yml"
@@ -155,6 +157,7 @@ def test_iwdr_permutations_singularity() -> None:
155157
[
156158
"--outdir",
157159
outdir,
160+
"--enable-dev",
158161
"--singularity",
159162
get_data(
160163
"tests/wf/iwdr_permutations.cwl"
@@ -199,6 +202,7 @@ def test_iwdr_permutations_singularity_inplace() -> None:
199202
outdir,
200203
"--singularity",
201204
"--enable-ext",
205+
"--enable-dev",
202206
"--overrides",
203207
get_data(
204208
"tests/wf/iwdr_permutations_inplace.yml"

‎tests/wf/iwdr_permutations.cwl‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#!/usr/bin/env cwl-runner
22
class: CommandLineTool
3-
cwlVersion: v1.0
3+
cwlVersion: v1.2.0-dev4
44
requirements:
55
DockerRequirement:
66
dockerPull: debian
@@ -74,4 +74,3 @@ arguments:
7474
echo "b" > /my_path/third_writable_file
7575
touch fifth_writable_directory/c
7676
touch /my_path/seventh_writable_directory/d
77-

0 commit comments

Comments
 (0)