Update: Here is my current corrected version (do not trust blindly, I had typos involved too!)

#!/usr/bin/env bash

TEMPDIR="$(mktemp -d)"
mkdir --verbose -- "${TEMPDIR}/tests"
trap 'cd -- "${TEMPDIR}/tests" && rm --verbose --one-file-system -rf "${TEMPDIR:-/invalid/615e1a5d}/tests"; cd ..; rmdir --verbose -- "${TEMPDIR}"' EXIT

And an alternative variant in case there are only files without subdirectories involved under “tests”:

trap 'cd -- "${TEMPDIR}/tests" && rm --verbose --one-file-system -f -- "${TEMPDIR:-/invalid/615e1a5d}/tests/"*; cd ..; rmdir --verbose -- tests "${TEMPDIR}"' EXIT

Note, I use --verbose to explicitly list files, because this is for my Test system. If you copy this construct to use in your own normal scripts, you might want to remove the verbose flags for normal usage.


Down below is old version:

This is just a little small question if this is secure. This script is used to create a fresh test environment that should get deleted when script ends. trap command solves that issue fine. However, I am very, very afraid of doing rm -rf in context of variables, in case the variable happens to become empty due to user error (or later changes in script). So I will do this in multiple steps.

#!/usr/bin/env bash

TEMPDIR="$(mktemp -d)"
mkdir -f -- "${TEMPDIR}/tests"
trap 'cd -- "${TEMPDIR}/tests" && rm -rf tests && cd .. && rmdir -- ${TEMPDIR}' EXIT

# Here follows the script content, creating temporary files and manipulating them...
  1. Use a subdirectory, so the variable is not used by itself. So we have to use ${TEMPDIR}/tests each time instead just ${TEMPDIR}.
  2. When removing all files recursively, first enter into directory with cd, and only if that was successful delete all files recursively with a specific directory name. This should make sure that rm -rf is only executed if the temporary directory even exist and the variable is not resolved to empty.
  3. Off course go up one dir again and then remove the empty directory with rmdir, which will only remove empty directories.

I personally feel confident that this construct is safe, but would like to hear your opinions. Maybe I missed something important. It would be devastating. I don’t want to try out various ways to see if one of them is working correctly.


Edit: For anyone who does not create uncontrolled temporary directories, they could just use rm -f tests/* instead, so nothing is deleted recursively. I may go that route and avoid sub-directories in my test folder.

  • thingsiplay@lemmy.mlOP
    link
    fedilink
    arrow-up
    3
    arrow-down
    1
    ·
    2 days ago

    Isn’t cd with && essentially doing that? Chain only runs, if TEMPDIR exists as a directory.

      • thingsiplay@lemmy.mlOP
        link
        fedilink
        arrow-up
        2
        arrow-down
        1
        ·
        2 days ago

        I did and it didn’t work. Lol, how did this happen? Well off course it does not work (but doesn’t delete anything), because I tried to enter into directory cd -- "${TEMPDIR}/tests" and then inside that directory tried to delete with rm -rf tests, which is empty at that point.

        So you see even if it looks perfectly valid…

    • mantricx@lemmy.world
      link
      fedilink
      arrow-up
      1
      arrow-down
      1
      ·
      2 days ago

      Yeah I think in practice. You seemed like you wanted extra paranoia steps. Also ran it through AI after posting and it called out mkdir doesn’t have a -f flag and you’ll need to cd just to TEMPDIR

      • thingsiplay@lemmy.mlOP
        link
        fedilink
        arrow-up
        1
        arrow-down
        1
        ·
        2 days ago

        Oh good catch. I meant to use mkdir -p, which is basically what -f to force in other commands is. It is actually not needed in this case anyway. I will also add --verbose to it.