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.

  • IanTwenty@piefed.social
    link
    fedilink
    English
    arrow-up
    1
    ·
    2 hours ago

    Supply mktemp with a suffix for the temp dir name, based on the pid your script runs as, so mktemp --suffix=$$.

    In your trap assert that the content of var TEMPDIR ends with your same current PID or fail before you clean anything up. You could also assert TEMPDIR is prefixed with $TMP or /tmp for even more robustness.

    This gives you a decent guarantee that TEMPDIR is what it should be and is the temp dir for THIS script run.

    • thingsiplay@lemmy.mlOP
      link
      fedilink
      arrow-up
      1
      ·
      edit-2
      5 minutes ago

      Edit: Please ignore this reply here, and read the discussion following the answers. I absolutely misunderstood the above reply.


      mktemp --suffix=$$ does not really solve the issue I have. I don’t want to use ${TEMPDIR}, but have something hardcoded when using the variable, as in rm -rf "${TEMPDIR}" vs rm -rf "${TEMPDIR}/tests". Because I’m not worried about what mktemp gives me back, but about when the variable is used at later time. The content of the variable could be altered after its initial creation.

      In your trap assert that the content of var TEMPDIR ends with your same current PID or fail before you clean anything up.

      I quiet don’t understand this point here. The trap command will run in any case the script exits, be a crash or normal exit. If the variable or directory is invalid, then the cleanup will not be executed. But that is by design, because I do not want to cleanup something that is not working correctly. I rather leave it to be cleaned up automatically with next reboot.

      You could also assert TEMPDIR is prefixed with $TMP or /tmp for even more robustness.

      It has already a fixed suffix part with “/tests” in use. I’m not worried about the TEMPDIR content if mktemp created it correctly. I’m more worried about the variable being altered and invalid at later point in the script. I would rather leave mktemp create the directory where it thinks is the best place (mostly it is /tmp, but that is not guaranteed). So prefixing the variable content itself doesn’t really solve the trust issues I have here, as it is not the creation time that I’m worried about.

      • IanTwenty@piefed.social
        link
        fedilink
        English
        arrow-up
        2
        ·
        37 minutes ago

        I think we misunderstand each other, let me try and be clearer myself. The line I suggest is:

        TEMPDIR=$(mktemp --suffix -$$)

        Which will look something like this when run:

        TEMPDIR=/tmp/tmp.9qRCIGOb2k-30978

        …if the pid is 30978 for example.

        In the trap we can then check if TEMPDIR has been overwritten or not with:

        trap ‘( [[ $TEMPDIR = /tmp/*-$$ ]] && rm -rf $TEMPDIR ) || echo “TEMPDIR var overwritten! Cleanup skipped.”’’ EXIT

        I am on my phone so forgive if any syntax is not quite right.

        • thingsiplay@lemmy.mlOP
          link
          fedilink
          arrow-up
          2
          ·
          10 minutes ago

          Ah I absolutely misunderstood you. The PID trick is actually clever!

          But I would still not solely want to rely on a $TEMPDIR variable alone, without a fixed path like "${TEMPDIR}/tests". The reason is, I am not just concerned about the trap cleanup, but also the usage in the script. I want never use the variable in the script (after setting up trap) without a fixed path on it. In example the script will change filenames, eventually using glob patterns or do other stuff. If the script is faulty and changes the temporary variable, then it will at least do this under “tests” no matter what.

          The idea is neat though. I have to think about this, experiment and see if I end up using this.

  • moonpiedumplings@programming.dev
    link
    fedilink
    English
    arrow-up
    2
    ·
    4 hours ago

    One workaround is that you could use the trash-cli command, which moves files to the trash directory instead of truly deleting them.

    But this only works on (usually desktop) systems that have a trash. And usually trash is set to autodelete items when it grows too big. I also am not sure if it’s always preinstalled.

    But it would be a neat workaround for worrying about permanently deleting files.

    • thingsiplay@lemmy.mlOP
      link
      fedilink
      arrow-up
      1
      ·
      4 hours ago

      I actually have trash-cli installed and exclusively use it for deleting all trash directories available on my system. Because I experience some inconsistencies how applications handle trash directories, involving mounted external drives.

      Moving files instead deleting them, in case they are important files is a good advice. However in case of temporary created and deleted files for testing software, I think this goes a bit too far. But it could prevent data loss, in case something goes wrong and I delete the wrong directory (hopefully the trash directory does not get deleted too, due to recursive deletion). Overall, this is a good advice to have in mind, I just think it goes a bit too far in this use case.

  • eleijeep@piefed.social
    link
    fedilink
    English
    arrow-up
    4
    ·
    5 hours ago

    Some solutions:

    set -e  
    TEMPDIR=$(mktemp -d)  
    # script exits if mktemp returns error  
    

    or

    TEMPDIR=$(mktemp -d || echo "/invalid")  
    # TEMPDIR gets the value "/invalid" if mktemp fails  
    

    or

    TEMPDIR=$(mktemp -d)  
    TEMPDIR=${TEMPDIR:-"/invalid"}  
    # TEMPDIR gets the value "/invalid" if mktemp returns an empty value  
    

    or

    TEMPDIR=$(mktemp -d)  
    rm -rf ${TEMPDIR:-"/invalid"}  
    # rm is passed "/invalid" if TEMPDIR is empty  
    

    My personal preference is the last one. Any time you call rm -rf you provide a default value to variables to ensure that if they are empty they get some other value instead.

    • thingsiplay@lemmy.mlOP
      link
      fedilink
      arrow-up
      2
      ·
      5 hours ago

      My reply is rejecting (most of) your suggestions, with reasons off course. I am glad you bring them up, so we can talk about it.

      I would avoid set -e option, as I do not want he entire script to exit on error. So instead I can use the exit command when I really want to on specific errors. I rather would like to handle errors myself directly, maybe even not exiting, but displaying error code with $? in example.

      TEMPDIR=$(mktemp -d || echo "/invalid")

      If anything, it would make more sense to just exit the script with || exit. In fact that is what I’m doing in the script after the trap command by cd "${TEMPDIR}/tests" || exit, so the script never continues without a successful mktemp directory.

      TEMPDIR=${TEMPDIR:-"/invalid"}

      I always forget that Bash has default values for variables! mktemp actually makes sure it never returns an empty value. I’m not worried about what it returns, but that my script could change the value of $TEMPDIR by accident (in example to something empty). So assigning a default value after mktemp will never have a chance to get the default value at all.

      rm -rf ${TEMPDIR:-"/invalid"}

      This on the other hand I like a lot. Now I will not stop doing my other additional checks, but for good habit this can’t be wrong. Maybe instead a custom directory name with an unlikely name, what about pointing it to /dev/null? I actually like this idea and might incorporate it.

      • eleijeep@piefed.social
        link
        fedilink
        English
        arrow-up
        1
        ·
        5 hours ago

        rm -rf ${TEMPDIR:-"/invalid"}
        This on the other hand I like a lot. Now I will not stop doing my other additional checks, but for good habit this can’t be wrong.

        I believe this is considered to be best practise, but I included the other options just for the sake of completeness.

        Happy to help.

        • thingsiplay@lemmy.mlOP
          link
          fedilink
          arrow-up
          1
          ·
          5 hours ago

          What if I would use /dev/null instead /invalid? Do you think this is a problem, better or worse? rm -rf ${TEMPDIR:-/dev/null}. I will update the current solution above, but need some research first. Edit: Oh wait, that could be dangerous if. If the script runs with root privileges, then /dev/null would be deleted.

          • eleijeep@piefed.social
            link
            fedilink
            English
            arrow-up
            2
            ·
            5 hours ago

            As a regular user that’s fine, but if your script might be run as root then there’s a possibility that you delete the special file /dev/null which would cause a great deal of problems for your system and probably be hard to debug if you don’t realise it has happened. I’ve heard of people doing this before so I think it is possible although I’ve never tried it.

            Edit: yeah I just saw your edit and I agree :)

    • thingsiplay@lemmy.mlOP
      link
      fedilink
      arrow-up
      4
      ·
      8 hours 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
          ·
          8 hours 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
        2
        ·
        8 hours 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
          ·
          8 hours 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.

  • TehPers@beehaw.org
    link
    fedilink
    English
    arrow-up
    2
    ·
    8 hours ago

    With Bash, you’ll only ever get “good enough” solutions. Even with your current setup, it’s susceptible to a race condition where another process adds to the TEMPDIR directory some other way during the script, and potentially even recreates tests after you delete it and before you remove the parent directory.

    Usually with Bash, the most readable solution is the best. I’d recommend a simple test for $TEMPDIR existing before a simple rm -rf "$TEMPDIR".

    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).

    In this case, just test for this? Test that the variable is not empty and that the directory exists, then rm -rf the directory. No need to overcomplicate it.

    • thingsiplay@lemmy.mlOP
      link
      fedilink
      arrow-up
      1
      ·
      7 hours ago

      it’s susceptible to a race condition where another process adds to TEMPDIR some other way during the script, and potentially even recreates tests after you delete it and before you remove the parent directory.

      You mean a subprocess from this script? In that case, the variable $TEMPDIR is never changed from the perspective of the script. Because its not exposed to subprocesses and the name is totally random by mktemp. So I don’t see how a subprocess should be able to do that.

      Usually with Bash, the most readable solution is the best. I’d recommend a simple test for $TEMPDIR existing before a simple rm -rf “$TEMPDIR”.

      This is what I want to avoid. Because what if I make mistakes in my own script and reassign $TEMPDIR by accident in a loop, instead reading from it. So at the time of execution of rm -rf, there is a chance that $TEMPDIR could potentially point to a different directory in example.

      • TehPers@beehaw.org
        link
        fedilink
        English
        arrow-up
        1
        ·
        7 hours ago

        So I don’t see how a subprocess should be able to do that.

        I’m referring to any process being able to do that, subprocess or not. If you know that no process on the system can interfere with your directory in any way, then you can be confident that nothing else will touch it.

        Because what if I make mistakes in my own script and reassign $TEMPDIR by accident in a loop, instead reading from it. So at the time of execution of rm -rf, there is a chance that $TEMPDIR could potentially point to a different directory in example.

        Create another variable for use in the script, and only use $TEMPDIR for creating and deleting the directory then. As long as you are certain you don’t reassign it, then you know the value won’t change, and you can use a second variable to ensure you don’t do that by accident.

        • thingsiplay@lemmy.mlOP
          link
          fedilink
          arrow-up
          1
          ·
          7 hours ago

          I’m referring to any process being able to do that, subprocess or not. If you know that no process on the system can interfere with your directory in any way, then you can be confident that nothing else will touch it.

          But how should any process know the variable of my script? mktemp makes sure its 100% random. And any process on the system can’t just read the variable out.

          • TehPers@beehaw.org
            link
            fedilink
            English
            arrow-up
            2
            ·
            7 hours ago

            But how should any process know the variable of my script?

            Processes can touch any directory they have access to. There can be any number of reasons that a process might do this, from antivirus software (which somehow exists on Linux) to search software leaving index files everywhere to something that just for some reason modifies random directories. My point was that Bash can’t guarantee that none of this ever happens, though rm -rf "$TEMPDIR" would get around that and delete the directory anyway (and any lingering contents).

            Since you only seem to be worried about accidentally deleting the wrong directory due to mistakes while presumably debugging, this isn’t really as relevant as I was suspecting it was. For something more robust though, I’d normally recommend a recursive delete without following any links (in case something links to files you don’t want to delete, like ~ or something).

            • thingsiplay@lemmy.mlOP
              link
              fedilink
              arrow-up
              1
              ·
              7 hours ago

              In case any process deletes files or the directory “$TEMPDIR” points to, the script should be covered, right? With cd -- "${TEMPDIR}/tests" it is guaranteed that the directory exists when running rm -rf command. And in case the “$TEMPDIR” variable is altered in any way (replaced or added home, ~ or any relative paths like …/…/…/home in example), at least having a hardcoded directory name with “tests” would make sure it never deletes anything under any circumstances that is not named “tests”.

              Unless symbolic links and other link files are involved and added to that directory.

  • FizzyOrange@programming.dev
    link
    fedilink
    arrow-up
    2
    arrow-down
    1
    ·
    8 hours ago

    You could do ./$TEMPDIR or better yet, just check it is still defined and non-empty.

    Or even betterer yet, don’t use Bash for something that you want to be robust. That’s like trying to build a life vest out of knives.

    • thingsiplay@lemmy.mlOP
      link
      fedilink
      arrow-up
      1
      ·
      8 hours ago

      ./$TEMPDIR

      That doesn’t solve my fear at all. “./” is still just relative. Because if in example “${TEMPDIR” happens to be empty, for whatever reason like wrong variable assignment or a typo, then “./${TEMPDIR” might resolve to “./” or any random directory that it has been assigned to. I would rather have a hardcoded name that is not just relative.

  • coolie4@lemmy.world
    link
    fedilink
    arrow-up
    1
    ·
    8 hours ago

    What is your specific use case for this? Is it you using rm -rf, and you’re afraid you’ll use it irresponsibly, or are you a sysadmin setting up an environment for someone else?

    In what way are file permissions not sufficient?

    • thingsiplay@lemmy.mlOP
      link
      fedilink
      arrow-up
      1
      ·
      8 hours ago

      It’s a script for testing another script or program. I want to set up a clean environment of files, that are created and deleted each time before running the test cases.

  • A_norny_mousse@piefed.zip
    link
    fedilink
    English
    arrow-up
    2
    arrow-down
    1
    ·
    8 hours ago

    I think you should start the last line with [ -d "$TEMPDIR" ] && cd ....

    Do you even need to use -f with rm?

    • thingsiplay@lemmy.mlOP
      link
      fedilink
      arrow-up
      1
      ·
      8 hours ago

      I think you should start the last line with [ -d “$TEMPDIR” ] && cd …

      So make sure directory exists, before cd into it? cd can only enter a directory that exists anyway, and the following && ensures following command runs only if directory exists and cd changed into it.

      Do you even need to use -f with rm?

      It depends if I create subdirectories in an uncontrolled manner. That was the plan, but I might change the plan. So just create temporary files only, then I would not need any recursive deletion.

    • thingsiplay@lemmy.mlOP
      link
      fedilink
      arrow-up
      1
      ·
      8 hours ago

      I usually avoid it too. But there are cases when its needed, like creating a clean test environment for test cases for scripts manipulating files and directories.

      • HubertManne@piefed.social
        link
        fedilink
        English
        arrow-up
        2
        ·
        8 hours ago

        yeah I just never came across it. Like I have used rm but don’t think I would ever do -f for sure no matter the case in a script. -r I don’t think I have used. Any cases where I made a directory I think it was just files in it and even then the files all had a set name scheme so I would rm based on the scheme for the files and then the directory seperately. Can’t say I have ever done all that complicated of scripts though. My big two was a user creation one and a disaster recovery nightly/weekly backup kind of thing.