From 98ab3fc242b9a5d1170f16f941ced6ef419b9fdc Mon Sep 17 00:00:00 2001
From: Ruben Vorderman <r.h.p.vorderman@lumc.nl>
Date: Wed, 31 Jul 2024 08:54:28 +0200
Subject: [PATCH 01/10] Set the singularity working directory to the job
 working directory

Previously the parameter was unused
---
 lib/galaxy/tool_util/deps/singularity_util.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/galaxy/tool_util/deps/singularity_util.py b/lib/galaxy/tool_util/deps/singularity_util.py
index 6d0044aa8cf1..5c1615a1552b 100644
--- a/lib/galaxy/tool_util/deps/singularity_util.py
+++ b/lib/galaxy/tool_util/deps/singularity_util.py
@@ -89,6 +89,8 @@ def build_singularity_run_command(
     )
     command_parts.append("-s")
     command_parts.append("exec")
+    if working_directory:
+        command_parts.extend(["--pwd", working_directory])
     if cleanenv:
         command_parts.append("--cleanenv")
     if no_mount:

From b1e5ee4397ad65970b8af63f0e05c2946c54f58f Mon Sep 17 00:00:00 2001
From: Ruben Vorderman <r.h.p.vorderman@lumc.nl>
Date: Wed, 31 Jul 2024 09:00:32 +0200
Subject: [PATCH 02/10] Run singularity containers with maximum isolation by
 default

---
 .../tool_util/deps/container_classes.py       |  3 +++
 lib/galaxy/tool_util/deps/singularity_util.py | 19 +++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/lib/galaxy/tool_util/deps/container_classes.py b/lib/galaxy/tool_util/deps/container_classes.py
index 769729f6bf9e..edcea0d44389 100644
--- a/lib/galaxy/tool_util/deps/container_classes.py
+++ b/lib/galaxy/tool_util/deps/container_classes.py
@@ -573,6 +573,9 @@ def containerize_command(self, command: str) -> str:
             guest_ports=self.tool_info.guest_ports,
             container_name=self.container_name,
             cleanenv=asbool(self.prop("cleanenv", singularity_util.DEFAULT_CLEANENV)),
+            ipc=asbool(self.prop("ipc", singularity_util.DEFAULT_IPC)),
+            pid=asbool(self.prop("pid", singularity_util.DEFAULT_PID)),
+            contain=asbool(self.prop("contain", singularity_util.DEFAULT_CONTAIN)),
             no_mount=self.prop("no_mount", singularity_util.DEFAULT_NO_MOUNT),
             **self.get_singularity_target_kwds(),
         )
diff --git a/lib/galaxy/tool_util/deps/singularity_util.py b/lib/galaxy/tool_util/deps/singularity_util.py
index 5c1615a1552b..3ad6a4c40f17 100644
--- a/lib/galaxy/tool_util/deps/singularity_util.py
+++ b/lib/galaxy/tool_util/deps/singularity_util.py
@@ -13,7 +13,17 @@
 
 DEFAULT_WORKING_DIRECTORY = None
 DEFAULT_SINGULARITY_COMMAND = "singularity"
+# --cleanenv --pid --ipc --contain is equivalent to the --containall flag.
+# This isolates a singularity container in the same way as a Docker container
+# would be isolated. --contain ensures no "default mounts" are mounted, the
+# default mounts include $HOME and that might affect reproducibility due to
+# settings files in $HOME. --pid isolates the pid namespace. --ipc isolates
+# the ipc namespace, this fixes some issues with python multiprocessing.
+# --cleanenv makes sure the current environment is not inherited.
 DEFAULT_CLEANENV = True
+DEFAULT_CONTAIN = True
+DEFAULT_IPC = True
+DEFAULT_PID = True
 DEFAULT_NO_MOUNT = ["tmp"]
 DEFAULT_SUDO = False
 DEFAULT_SUDO_COMMAND = "sudo"
@@ -71,6 +81,9 @@ def build_singularity_run_command(
     guest_ports: Union[bool, List[str]] = False,
     container_name: Optional[str] = None,
     cleanenv: bool = DEFAULT_CLEANENV,
+    ipc: bool = DEFAULT_IPC,
+    pid: bool = DEFAULT_PID,
+    contain: bool = DEFAULT_CONTAIN,
     no_mount: Optional[List[str]] = DEFAULT_NO_MOUNT,
 ) -> str:
     volumes = volumes or []
@@ -93,6 +106,12 @@ def build_singularity_run_command(
         command_parts.extend(["--pwd", working_directory])
     if cleanenv:
         command_parts.append("--cleanenv")
+    if ipc:
+        command_parts.append("--ipc")
+    if pid:
+        command_parts.append("--pid")
+    if contain:
+        command_parts.append("--contain")
     if no_mount:
         command_parts.extend(["--no-mount", ",".join(no_mount)])
     for volume in volumes:

From eb31c510b609bf48c0b9f10f15638df72bcd8084 Mon Sep 17 00:00:00 2001
From: Ruben Vorderman <r.h.p.vorderman@lumc.nl>
Date: Wed, 31 Jul 2024 09:06:07 +0200
Subject: [PATCH 03/10] Singularity containers only mount home directory when
 singularity_mount_home is true

---
 lib/galaxy/tool_util/deps/container_classes.py | 1 +
 lib/galaxy/tool_util/deps/singularity_util.py  | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/galaxy/tool_util/deps/container_classes.py b/lib/galaxy/tool_util/deps/container_classes.py
index edcea0d44389..8891c95af830 100644
--- a/lib/galaxy/tool_util/deps/container_classes.py
+++ b/lib/galaxy/tool_util/deps/container_classes.py
@@ -576,6 +576,7 @@ def containerize_command(self, command: str) -> str:
             ipc=asbool(self.prop("ipc", singularity_util.DEFAULT_IPC)),
             pid=asbool(self.prop("pid", singularity_util.DEFAULT_PID)),
             contain=asbool(self.prop("contain", singularity_util.DEFAULT_CONTAIN)),
+            mount_home=asbool(self.prop("mount_home", singularity_util.DEFAULT_MOUNT_HOME)),
             no_mount=self.prop("no_mount", singularity_util.DEFAULT_NO_MOUNT),
             **self.get_singularity_target_kwds(),
         )
diff --git a/lib/galaxy/tool_util/deps/singularity_util.py b/lib/galaxy/tool_util/deps/singularity_util.py
index 3ad6a4c40f17..ebeb9922329e 100644
--- a/lib/galaxy/tool_util/deps/singularity_util.py
+++ b/lib/galaxy/tool_util/deps/singularity_util.py
@@ -24,6 +24,7 @@
 DEFAULT_CONTAIN = True
 DEFAULT_IPC = True
 DEFAULT_PID = True
+DEFAULT_MOUNT_HOME = False
 DEFAULT_NO_MOUNT = ["tmp"]
 DEFAULT_SUDO = False
 DEFAULT_SUDO_COMMAND = "sudo"
@@ -84,6 +85,7 @@ def build_singularity_run_command(
     ipc: bool = DEFAULT_IPC,
     pid: bool = DEFAULT_PID,
     contain: bool = DEFAULT_CONTAIN,
+    mount_home: bool = DEFAULT_MOUNT_HOME,
     no_mount: Optional[List[str]] = DEFAULT_NO_MOUNT,
 ) -> str:
     volumes = volumes or []
@@ -116,7 +118,7 @@ def build_singularity_run_command(
         command_parts.extend(["--no-mount", ",".join(no_mount)])
     for volume in volumes:
         command_parts.extend(["-B", str(volume)])
-    if home is not None:
+    if mount_home and home is not None:
         command_parts.extend(["--home", f"{home}:{home}"])
     if run_extra_arguments:
         command_parts.append(run_extra_arguments)

From d6e7c538eeba392b19a291ea03ed216e21024229 Mon Sep 17 00:00:00 2001
From: Ruben Vorderman <r.h.p.vorderman@lumc.nl>
Date: Wed, 31 Jul 2024 09:18:21 +0200
Subject: [PATCH 04/10] Add documentation for new singularity settings

---
 .../config/sample/job_conf.xml.sample_advanced | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/lib/galaxy/config/sample/job_conf.xml.sample_advanced b/lib/galaxy/config/sample/job_conf.xml.sample_advanced
index dc67a4763d74..d143d0b72336 100644
--- a/lib/galaxy/config/sample/job_conf.xml.sample_advanced
+++ b/lib/galaxy/config/sample/job_conf.xml.sample_advanced
@@ -629,6 +629,24 @@
                argument by default. You can turn this off by setting singularity_cleanenv to `false`.
           -->
           <!-- <param id="singularity_cleanenv">true</param> -->
+          <!-- Singularity by default inherits the PID namespace, this can give
+               issues with multiprocessing, hence galaxy passes the `pid` argument
+               by default to isolate the PID namespace. You can turn this of by setting
+               singularity_pid to `false`.
+          -->
+          <!-- <param id="singularity_pid">true</param> -->
+          <!-- Singularity by default inherits the IPC namespace, this can give
+               issues with multiprocessing, hence galaxy passes the `ipc` argument
+               by default to isolate the PID namespace. You can turn this of by setting
+               singularity_ipc to `false`.
+          -->
+          <!-- <param id="singularity_ipc">true</param> -->
+          <!-- Singularity mounts $HOME by default. This can be problematic if home
+               contains sensitive data, is shared across nodes or has configuration
+               files affecting reproducibility. Therefore Galaxy passes the `contain`
+               argument to isolate $HOME. If $HOME must be mounted, it can be
+               turned on by setting singularity_mount_home to `true`. -->
+          <!-- <param id="singularity_mount_home">false</param> -->
           <!-- Pass extra arguments to the singularity exec command not covered by the
                above options. -->
           <!-- <param id="singularity_run_extra_arguments"></param> -->

From 4c5a860b8d9dd5ecb221da522394d0d480ec61cd Mon Sep 17 00:00:00 2001
From: Ruben Vorderman <r.h.p.vorderman@lumc.nl>
Date: Wed, 31 Jul 2024 09:24:55 +0200
Subject: [PATCH 05/10] Document that tmp_dir should be set to true for proper
 singularity container work

---
 lib/galaxy/config/sample/job_conf.xml.sample_advanced | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/galaxy/config/sample/job_conf.xml.sample_advanced b/lib/galaxy/config/sample/job_conf.xml.sample_advanced
index d143d0b72336..b91f6e5425a7 100644
--- a/lib/galaxy/config/sample/job_conf.xml.sample_advanced
+++ b/lib/galaxy/config/sample/job_conf.xml.sample_advanced
@@ -604,6 +604,12 @@
         </destination>
         <destination id="singularity_local" runner="local">
           <param id="singularity_enabled">true</param>
+          <!--Galaxy requests an isolated /tmp directory from singularity, which means
+              it will be mounted as read-only. This can cause some problems with tools that
+              do not use the TMP, TEMP, TMPDIR variable family properly. Setting
+              tmp_dir to true ensures that the /tmp directory in the container points
+              to the job specific working tmp directory.-->
+          <param id="tmp_dir">true</param>
           <!-- See the above documentation for docker_volumes, singularity_volumes works
                almost the same way. The only difference is that $default will expand with
                rw directories that in Docker would expand as ro if any of subdirectories are rw.

From 74e5612742060bd28b75d7ff5aa9b4c55bb0b4ec Mon Sep 17 00:00:00 2001
From: Ruben Vorderman <r.h.p.vorderman@lumc.nl>
Date: Wed, 31 Jul 2024 09:11:43 +0200
Subject: [PATCH 06/10] Remove redundant --contain switch

---
 lib/galaxy/tool_util/deps/container_classes.py | 1 -
 lib/galaxy/tool_util/deps/singularity_util.py  | 9 +++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/galaxy/tool_util/deps/container_classes.py b/lib/galaxy/tool_util/deps/container_classes.py
index 8891c95af830..55acaae81b86 100644
--- a/lib/galaxy/tool_util/deps/container_classes.py
+++ b/lib/galaxy/tool_util/deps/container_classes.py
@@ -575,7 +575,6 @@ def containerize_command(self, command: str) -> str:
             cleanenv=asbool(self.prop("cleanenv", singularity_util.DEFAULT_CLEANENV)),
             ipc=asbool(self.prop("ipc", singularity_util.DEFAULT_IPC)),
             pid=asbool(self.prop("pid", singularity_util.DEFAULT_PID)),
-            contain=asbool(self.prop("contain", singularity_util.DEFAULT_CONTAIN)),
             mount_home=asbool(self.prop("mount_home", singularity_util.DEFAULT_MOUNT_HOME)),
             no_mount=self.prop("no_mount", singularity_util.DEFAULT_NO_MOUNT),
             **self.get_singularity_target_kwds(),
diff --git a/lib/galaxy/tool_util/deps/singularity_util.py b/lib/galaxy/tool_util/deps/singularity_util.py
index ebeb9922329e..1eee5abdb74f 100644
--- a/lib/galaxy/tool_util/deps/singularity_util.py
+++ b/lib/galaxy/tool_util/deps/singularity_util.py
@@ -21,7 +21,6 @@
 # the ipc namespace, this fixes some issues with python multiprocessing.
 # --cleanenv makes sure the current environment is not inherited.
 DEFAULT_CLEANENV = True
-DEFAULT_CONTAIN = True
 DEFAULT_IPC = True
 DEFAULT_PID = True
 DEFAULT_MOUNT_HOME = False
@@ -84,7 +83,6 @@ def build_singularity_run_command(
     cleanenv: bool = DEFAULT_CLEANENV,
     ipc: bool = DEFAULT_IPC,
     pid: bool = DEFAULT_PID,
-    contain: bool = DEFAULT_CONTAIN,
     mount_home: bool = DEFAULT_MOUNT_HOME,
     no_mount: Optional[List[str]] = DEFAULT_NO_MOUNT,
 ) -> str:
@@ -104,6 +102,11 @@ def build_singularity_run_command(
     )
     command_parts.append("-s")
     command_parts.append("exec")
+    # Singularity mounts some directories, such as $HOME and $PWD by default.
+    # using --contain disables this behaviour and only allows explicitly
+    # requested volumes to be mounted. Since galaxy already mounts $PWD and
+    # mount_home can be activated, a switch for --contain is redundant.
+    command_parts.append("--contain")
     if working_directory:
         command_parts.extend(["--pwd", working_directory])
     if cleanenv:
@@ -112,8 +115,6 @@ def build_singularity_run_command(
         command_parts.append("--ipc")
     if pid:
         command_parts.append("--pid")
-    if contain:
-        command_parts.append("--contain")
     if no_mount:
         command_parts.extend(["--no-mount", ",".join(no_mount)])
     for volume in volumes:

From dc07e5061e3d1387ebf7daa6f3a29915e1181c89 Mon Sep 17 00:00:00 2001
From: Ruben Vorderman <r.h.p.vorderman@lumc.nl>
Date: Wed, 31 Jul 2024 09:53:03 +0200
Subject: [PATCH 07/10] fix tests for singularity

---
 test/unit/tool_util/test_singularity_util.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/test/unit/tool_util/test_singularity_util.py b/test/unit/tool_util/test_singularity_util.py
index 2399a70a91b6..dc460285f520 100644
--- a/test/unit/tool_util/test_singularity_util.py
+++ b/test/unit/tool_util/test_singularity_util.py
@@ -6,13 +6,15 @@ def test_build_singularity_run_command_defaults():
         container_command="echo hi",
         image="busybox",
     )
-    assert cmd == "singularity -s exec --cleanenv --no-mount tmp busybox echo hi"
+    assert cmd == "singularity -s exec --contain --cleanenv --ipc --pid --no-mount tmp busybox echo hi"
 
 
-def test_build_singularity_run_command_no_cleanenv():
+def test_build_singularity_run_command_no_cleanenv_ipc_pid():
     cmd = build_singularity_run_command(
         container_command="echo hi",
         image="busybox",
         cleanenv=False,
+        pid=False,
+        ipc=False,
     )
-    assert cmd == "singularity -s exec --no-mount tmp busybox echo hi"
+    assert cmd == "singularity -s exec --contain --no-mount tmp busybox echo hi"

From 7cff7b47af9274714d7fb0ce27a72f78d43ba5ef Mon Sep 17 00:00:00 2001
From: Ruben Vorderman <r.h.p.vorderman@lumc.nl>
Date: Mon, 5 Aug 2024 11:10:30 +0200
Subject: [PATCH 08/10] Remove erroneous mount home functionality

This should be determined by the tool profile
---
 lib/galaxy/config/sample/job_conf.xml.sample_advanced | 6 ------
 lib/galaxy/tool_util/deps/container_classes.py        | 1 -
 lib/galaxy/tool_util/deps/singularity_util.py         | 7 +++----
 3 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/lib/galaxy/config/sample/job_conf.xml.sample_advanced b/lib/galaxy/config/sample/job_conf.xml.sample_advanced
index b91f6e5425a7..0ae9c855d3be 100644
--- a/lib/galaxy/config/sample/job_conf.xml.sample_advanced
+++ b/lib/galaxy/config/sample/job_conf.xml.sample_advanced
@@ -647,12 +647,6 @@
                singularity_ipc to `false`.
           -->
           <!-- <param id="singularity_ipc">true</param> -->
-          <!-- Singularity mounts $HOME by default. This can be problematic if home
-               contains sensitive data, is shared across nodes or has configuration
-               files affecting reproducibility. Therefore Galaxy passes the `contain`
-               argument to isolate $HOME. If $HOME must be mounted, it can be
-               turned on by setting singularity_mount_home to `true`. -->
-          <!-- <param id="singularity_mount_home">false</param> -->
           <!-- Pass extra arguments to the singularity exec command not covered by the
                above options. -->
           <!-- <param id="singularity_run_extra_arguments"></param> -->
diff --git a/lib/galaxy/tool_util/deps/container_classes.py b/lib/galaxy/tool_util/deps/container_classes.py
index 55acaae81b86..e00c005e5275 100644
--- a/lib/galaxy/tool_util/deps/container_classes.py
+++ b/lib/galaxy/tool_util/deps/container_classes.py
@@ -575,7 +575,6 @@ def containerize_command(self, command: str) -> str:
             cleanenv=asbool(self.prop("cleanenv", singularity_util.DEFAULT_CLEANENV)),
             ipc=asbool(self.prop("ipc", singularity_util.DEFAULT_IPC)),
             pid=asbool(self.prop("pid", singularity_util.DEFAULT_PID)),
-            mount_home=asbool(self.prop("mount_home", singularity_util.DEFAULT_MOUNT_HOME)),
             no_mount=self.prop("no_mount", singularity_util.DEFAULT_NO_MOUNT),
             **self.get_singularity_target_kwds(),
         )
diff --git a/lib/galaxy/tool_util/deps/singularity_util.py b/lib/galaxy/tool_util/deps/singularity_util.py
index 1eee5abdb74f..cd4d69161a54 100644
--- a/lib/galaxy/tool_util/deps/singularity_util.py
+++ b/lib/galaxy/tool_util/deps/singularity_util.py
@@ -83,7 +83,6 @@ def build_singularity_run_command(
     cleanenv: bool = DEFAULT_CLEANENV,
     ipc: bool = DEFAULT_IPC,
     pid: bool = DEFAULT_PID,
-    mount_home: bool = DEFAULT_MOUNT_HOME,
     no_mount: Optional[List[str]] = DEFAULT_NO_MOUNT,
 ) -> str:
     volumes = volumes or []
@@ -104,8 +103,8 @@ def build_singularity_run_command(
     command_parts.append("exec")
     # Singularity mounts some directories, such as $HOME and $PWD by default.
     # using --contain disables this behaviour and only allows explicitly
-    # requested volumes to be mounted. Since galaxy already mounts $PWD and
-    # mount_home can be activated, a switch for --contain is redundant.
+    # requested volumes to be mounted. This gives fully full-control over
+    # the mounting behavior.
     command_parts.append("--contain")
     if working_directory:
         command_parts.extend(["--pwd", working_directory])
@@ -119,7 +118,7 @@ def build_singularity_run_command(
         command_parts.extend(["--no-mount", ",".join(no_mount)])
     for volume in volumes:
         command_parts.extend(["-B", str(volume)])
-    if mount_home and home is not None:
+    if home is not None:
         command_parts.extend(["--home", f"{home}:{home}"])
     if run_extra_arguments:
         command_parts.append(run_extra_arguments)

From 639fd541f654b29a77e627c0e5cde79a55d57e5c Mon Sep 17 00:00:00 2001
From: Ruben Vorderman <r.h.p.vorderman@lumc.nl>
Date: Mon, 5 Aug 2024 11:12:46 +0200
Subject: [PATCH 09/10] Remove redundant comment and default

---
 lib/galaxy/tool_util/deps/singularity_util.py | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/lib/galaxy/tool_util/deps/singularity_util.py b/lib/galaxy/tool_util/deps/singularity_util.py
index cd4d69161a54..2d090fc6237c 100644
--- a/lib/galaxy/tool_util/deps/singularity_util.py
+++ b/lib/galaxy/tool_util/deps/singularity_util.py
@@ -13,17 +13,12 @@
 
 DEFAULT_WORKING_DIRECTORY = None
 DEFAULT_SINGULARITY_COMMAND = "singularity"
-# --cleanenv --pid --ipc --contain is equivalent to the --containall flag.
-# This isolates a singularity container in the same way as a Docker container
-# would be isolated. --contain ensures no "default mounts" are mounted, the
-# default mounts include $HOME and that might affect reproducibility due to
-# settings files in $HOME. --pid isolates the pid namespace. --ipc isolates
+# --pid isolates the pid namespace. --ipc isolates
 # the ipc namespace, this fixes some issues with python multiprocessing.
 # --cleanenv makes sure the current environment is not inherited.
 DEFAULT_CLEANENV = True
 DEFAULT_IPC = True
 DEFAULT_PID = True
-DEFAULT_MOUNT_HOME = False
 DEFAULT_NO_MOUNT = ["tmp"]
 DEFAULT_SUDO = False
 DEFAULT_SUDO_COMMAND = "sudo"

From a66b09cf1e6e738e106bca2fa8571a544640db8d Mon Sep 17 00:00:00 2001
From: Ruben Vorderman <r.h.p.vorderman@lumc.nl>
Date: Wed, 7 Aug 2024 07:19:25 +0200
Subject: [PATCH 10/10] shlex quote the working directory

Co-authored-by: Nicola Soranzo <nicola.soranzo@gmail.com>
---
 lib/galaxy/tool_util/deps/singularity_util.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/galaxy/tool_util/deps/singularity_util.py b/lib/galaxy/tool_util/deps/singularity_util.py
index 2d090fc6237c..081c706fc3c4 100644
--- a/lib/galaxy/tool_util/deps/singularity_util.py
+++ b/lib/galaxy/tool_util/deps/singularity_util.py
@@ -102,7 +102,7 @@ def build_singularity_run_command(
     # the mounting behavior.
     command_parts.append("--contain")
     if working_directory:
-        command_parts.extend(["--pwd", working_directory])
+        command_parts.extend(["--pwd", shlex.quote(working_directory)])
     if cleanenv:
         command_parts.append("--cleanenv")
     if ipc: