3
\$\begingroup\$

For a research analysis, I'm writing a small bash script that serves as a frontend to Singularity. The reason is that I want to save in this script which options are needed for singularity. For example, I want the working directory to appear at a fixed path in the container, regardless of the actual working directory path.

Singularity has the --contain option for this, but this requires me to manually define a temporary directory for the container. I use mktemp for that. After the end of the script, I use trap to delete that directory. However, I fear that there might be a corner case where trap "rm -rf '$tmpdir' might delete the wrong directory.

The script, called cexec, executes an arbitrary command inside the container. For example ./cexec R starts R inside the container.

Is there a corner case in the following script where the script deletes a directory that it didn't create?

#!/bin/bash
# Execute a command in the container
set -ue

thisdir="$(dirname "$BASH_SOURCE")"
container="rserver_200211_commitd117c677.sif" # get such a file by using `singularity pull ...`

# Create a temporary directory
tmpdir="$(mktemp -d -t cexec-XXXXXXXX)"
# We delete this directory afterwards, so its important that $tmpdir
# really has the path to an empty, temporary dir, and nothing else!
# (for example empty string or home dir)
if [[ ! "$tmpdir" || ! -d "$tmpdir" ]]; then
  echo "Error: Could not create temp dir $tmpdir"
  exit 1
fi
# check if temp dir is empty
tmpcontent="$(ls -A "$tmpdir")"
if [ ! -z "$tmpcontent" ]; then
  echo "Error: Temp dir '$tmpdir' is not empty"
  exit 1
fi
# Delete the temporary directory after the end of the script
trap "rm -rf '$tmpdir'" EXIT

singularity exec \
  -B "$tmpdir:/tmp" \
  --contain \
  -H "$thisdir:/data" \
  "$container" \
  "$@"
\$\endgroup\$
2
\$\begingroup\$

Always set up the trap before trying to create the directory. Otherwise there's a race condition where the script may die after creating the directory but before having a chance to clean it up. And use single quotes for the trap command to make sure the variable is only expanded at exit:

trap 'rm -rf "$tmpdir"' EXIT
tmpdir="$(mktemp -d)"

If the trap ends up being triggered before mktemp the result is simply rm -rf "", which does nothing.

Some other suggestions:

  1. #!/usr/bin/env bash is a more portable shebang.
  2. set -o errexit -o nounset is more readable than set -ue.
  3. BASH_SOURCE is an array, and it's only by accident that $array_name refers to the first element of array_name. directory="$(dirname "${BASH_SOURCE[0]}")" would be more explicit.
  4. if [[ ! "$tmpdir" || ! -d "$tmpdir" ]] is redundant. If mktemp fails the script will stop there, and if it's been removed rm -rf "$tmpdir" is safe, as mentioned.
  5. Don't use ls in scripts!
  6. The temporary directory can't already contain any files. That's part of the contract of mktemp - it will create a new directory if it can, or fail otherwise.
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I don't normally like single quotes for the trap, leaving the variable unexpanded (there's a risk that some other code modifies it, particularly with a generic name like that). So I'd have something like while tmpdir="$(mktemp -u)"; trap "rm -rf '$tmpdir'" EXIT; mkdir "$tmpdir"; do : ; done \$\endgroup\$ – Toby Speight Mar 6 at 10:58
  • \$\begingroup\$ If you're worried about something modifying tmpdir you can just readonly tmpdir. \$\endgroup\$ – l0b0 Mar 6 at 20:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.