bug/1480: make writable workdir actually writable (#1481)

Co-authored-by: Michael R. Crusoe <[email protected]>

Author: John Didion

cwltool/process.py

@@ -308,7 +308,7 @@ def stage_files(
                 os.makedirs(entry.target)
             else:
                 shutil.copytree(entry.resolved, entry.target)
-                ensure_writable(entry.target)
+                ensure_writable(entry.target, include_root=True)
         elif entry.type == "CreateFile" or entry.type == "CreateWritableFile":
             with open(entry.target, "wb") as new:
                 if secret_store is not None:

cwltool/utils.py

@@ -367,23 +367,30 @@ def downloadHttpFile(httpurl):
     return str(f.name)
 
 
-def ensure_writable(path):  # type: (str) -> None
+def ensure_writable(path: str, include_root: bool = False) -> None:
+    """
+    Ensure that 'path' is writable.
+
+    If 'path' is a directory, then all files and directories under 'path' are
+    made writable, recursively. If 'path' is a file or if 'include_root' is
+    `True`, then 'path' itself is made writable.
+    """
+
+    def add_writable_flag(p: str) -> None:
+        st = os.stat(p)
+        mode = stat.S_IMODE(st.st_mode)
+        os.chmod(p, mode | stat.S_IWUSR)
+
     if os.path.isdir(path):
+        if include_root:
+            add_writable_flag(path)
         for root, dirs, files in os.walk(path):
             for name in files:
-                j = os.path.join(root, name)
-                st = os.stat(j)
-                mode = stat.S_IMODE(st.st_mode)
-                os.chmod(j, mode | stat.S_IWUSR)
+                add_writable_flag(os.path.join(root, name))
             for name in dirs:
-                j = os.path.join(root, name)
-                st = os.stat(j)
-                mode = stat.S_IMODE(st.st_mode)
-                os.chmod(j, mode | stat.S_IWUSR)
+                add_writable_flag(os.path.join(root, name))
     else:
-        st = os.stat(path)
-        mode = stat.S_IMODE(st.st_mode)
-        os.chmod(path, mode | stat.S_IWUSR)
+        add_writable_flag(path)
 
 
 def ensure_non_writable(path):  # type: (str) -> None

tests/test_iwdr.py

@@ -1,4 +1,5 @@
 from pathlib import Path
+from stat import S_IWRITE, S_IWGRP, S_IWOTH
 from typing import Any
 
 from cwltool.factory import Factory
@@ -80,6 +81,53 @@ def test_iwdr_permutations(tmp_path_factory: Any) -> None:
     )
 
 
+def test_iwdr_permutations_readonly(tmp_path_factory: Any) -> None:
+    """Confirm that readonly input files are properly made writable."""
+    misc = tmp_path_factory.mktemp("misc")
+    fifth = tmp_path_factory.mktemp("fifth")
+    fifth_file = fifth / "bar"
+    fifth_dir = fifth / "foo"
+    fifth_file.touch()
+    fifth_dir.mkdir()
+    sixth = tmp_path_factory.mktemp("sixth")
+    first = misc / "first"
+    first.touch()
+    second = misc / "second"
+    second.touch()
+    outdir = str(tmp_path_factory.mktemp("outdir"))
+    for entry in [first, second, fifth, sixth, fifth_file, fifth_dir]:
+        mode = entry.stat().st_mode
+        ro_mask = 0o777 ^ (S_IWRITE | S_IWGRP | S_IWOTH)
+        entry.chmod(mode & ro_mask)
+    assert (
+        main(
+            [
+                "--no-container",
+                "--debug",
+                "--leave-outputs",
+                "--outdir",
+                outdir,
+                get_data("tests/wf/iwdr_permutations_nocontainer.cwl"),
+                "--first",
+                str(first),
+                "--second",
+                str(second),
+                "--fifth",
+                str(fifth),
+                "--sixth",
+                str(sixth),
+            ]
+        )
+        == 0
+    )
+    for entry in [first, second, fifth, sixth, fifth_file, fifth_dir]:
+        try:
+            mode = entry.stat().st_mode
+            entry.chmod(mode | S_IWRITE)
+        except PermissionError:
+            pass
+
+
 @needs_docker
 def test_iwdr_permutations_inplace(tmp_path_factory: Any) -> None:
     misc = tmp_path_factory.mktemp("misc")

tests/wf/iwdr_permutations_nocontainer.cwl

@@ -0,0 +1,43 @@
+#!/usr/bin/env cwl-runner
+class: CommandLineTool
+cwlVersion: v1.2
+requirements:
+  InitialWorkDirRequirement:
+    listing:
+      - entry: $(inputs.first)
+        entryname: first_writable_file
+        writable: true
+      - entry: $(inputs.second)
+        entryname: second_read_only_file
+        writable: false
+      - entry: $(inputs.fifth)
+        entryname: fifth_writable_directory
+        writable: true
+      - entry: $(inputs.sixth)
+        entryname: sixth_read_only_directory
+        writable: false
+      - entry: $(inputs.ninth)
+        entryname: nineth_writable_directory_literal
+        writable: true
+inputs:
+  first: File
+  second: File
+  fifth: Directory
+  sixth: Directory
+  ninth:
+    type: Directory
+    default:
+      class: Directory
+      basename: foo
+      listing: []
+outputs:
+  out:
+    type: Directory
+    outputBinding:
+      glob: .
+baseCommand: [bash, -c]
+arguments:
+  - |
+    find .
+    echo "a" > first_writable_file
+    touch fifth_writable_directory/c