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.

  • FizzyOrange@programming.dev
    link
    fedilink
    arrow-up
    3
    ·
    2 months 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
      ·
      2 months 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.

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

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

      • mantricx@lemmy.world
        link
        fedilink
        arrow-up
        1
        arrow-down
        1
        ·
        2 months 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
          ·
          2 months 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.

  • ThanksForAllTheFish@sh.itjust.works
    link
    fedilink
    arrow-up
    2
    arrow-down
    1
    ·
    2 months ago

    This might go beyond what you were asking for, but I’m always more comfortable handling file paths, deletions and errors in Python.

    #!/usr/bin/env python3
    
    import shutil
    from pathlib import Path
    from tempfile import TemporaryDirectory
    
    # Refuse to run without Python's symlink-resistant cleanup.
    if not getattr(shutil.rmtree, "avoids_symlink_attacks", False):
        raise RuntimeError("Symlink-resistant cleanup is unavailable")
    
    with TemporaryDirectory(prefix="test-run-", dir="/tmp") as name:
        tests = Path(name) / "tests"
        tests.mkdir(mode=0o700)
        print(f"Created {str(tests)!r}", flush=True)
    
        try:
            # Put your test code here.
            (tests / "example.txt").write_text(
                "temporary data\n", encoding="utf-8"
            )
        finally:
            print(f"Cleaning {name!r}", flush=True)
    
    # The entire temporary directory has now been removed.
    
    • thingsiplay@lemmy.mlOP
      link
      fedilink
      arrow-up
      1
      ·
      2 months ago

      Funny enough I’m writing (and wrote before) a Python script that is all about files and renaming. And this Bash script is about creating a test environment. Maybe its actually a good idea to write this in Python too.

  • coolie4@lemmy.world
    link
    fedilink
    arrow-up
    1
    ·
    2 months 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
      ·
      2 months 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.

  • MadhuGururajan@programming.dev
    link
    fedilink
    English
    arrow-up
    1
    ·
    2 months ago

    good usecase for find … -delete. Test it out on a trial directory and it’s should work. You can set -maxdepth if you are concerned about sumlinks… but i bet there is an option to not follow symlinks

  • arthropod_shift@programming.dev
    link
    fedilink
    arrow-up
    2
    arrow-down
    1
    ·
    2 months ago

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

    I get the feeling that either there’s some missing info, a misunderstanding on my part, or there might be a simpler way to do things. Is $TEMPDIR changing a reasonable case to handle? Can you change your script design to make this impossible instead? (Like replacing a source with a script execution?)

    If the snippet is just boilerplate at the top of numerous scripts, I’d do set -u, rm -rf "${TEMPDIR}" on exit (ideally defined in a common setup script/function), and just avoid assigning to the var later in the script. In terms of defensive programming, anything extra is added complexity that will only make an error more likely imo. You could rename TEMPDIR to something like TEST_ROOT if you’re concerned about the variable name being accidentally used again, but no amount of trap logic is going to make a future programming error impossible.

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

      I know, good habit, check variable and so on. But mistakes happen, so what’s wrong with hardening the case? There are decades of studies and good practice for coding, yet the best programmers still do mistakes. And having a script that could potentially delete files on your system (even all subdirectories) is dangerous and should be handled with respect. I really don’t understand the opposition here.

      In terms of defensive programming, anything extra is added complexity that will only make an error more likely imo.

      I disagree here. Adding checks will help in catching those errors. Being not defensive about it will make it only more likely to make errors.

      no amount of trap logic is going to make a future programming error impossible.

      No amount of any programming will make it impossible to error out. That does not mean we shouldn’t try to make it as secure as we can think. Renaming the variable is putting the risk to another name, not really solving the issue.

      • arthropod_shift@programming.dev
        link
        fedilink
        arrow-up
        1
        arrow-down
        1
        ·
        2 months ago

        I think you mistook my asking questions as opposition, or maybe my intent wasn’t clear enough in my comment. My main point was that while I don’t have full context for the problem, it seems to me that a simpler solution exists. Is there some edge case that the cd -> rm -rf -> rmdir snippet covers that’s missed by set -u -> rm -rf?

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

          I generally dislike using the set methods to change how the “language” works. So even if it covers my issue, I’m not using it. Its also not even said that these options couldn’t be changed, in fact I think in some cases it can be useful to change those options temporarily for certain effects like “pipefail” or the one that prints the executed lines. But not as a default or for critical commands that can run at any time the script exits (even on error). Maybe someone (even me) copy pastes this line in example.

          I always forget which of these set options do what, and next time when I write or read another script it could have different set of options. So I ignore those set options to change how the language is interpreted in Bash. Also if cd -> rm -rf -> rmdir solves it, why would I need to rewrite and change it to set -u -> rm -rf?

          • arthropod_shift@programming.dev
            link
            fedilink
            arrow-up
            2
            ·
            2 months ago

            I generally dislike using the set methods to change how the “language” works. So even if it covers my issue, I’m not using it.

            If you don’t feel like using it, that’s valid, just as long as you’re aware of the functionality. It seemed fitting here to me because of the potential to simplify the code from a complex chain of commands with hard-coded values to a single command with no hard-coding, while keeping the old behavior.

            Also if cd -> rm -rf -> rmdir solves it, why would I need to rewrite and change it to set -u -> rm -rf?

            You don’t. You asked for feedback, though, so I gave some.

            In general, I’m a fan of set -u because it helps to avoid some common scripting bugs, but if you’re aware of the option and how it could be used, and you choose not to use it, then I’m not going to insist you write it how I would write it.

  • TehPers@beehaw.org
    link
    fedilink
    English
    arrow-up
    2
    arrow-down
    1
    ·
    2 months 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
      ·
      2 months 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
        2
        arrow-down
        1
        ·
        2 months 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
          ·
          2 months 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
            1
            arrow-down
            1
            ·
            2 months 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
              ·
              2 months 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.

  • mvirts@lemmy.world
    link
    fedilink
    arrow-up
    1
    arrow-down
    1
    ·
    2 months ago

    A silly idea if you want to remove rm entirely:

    fallocate a file (or use dd) Mkfs the file to your favorite filesystem Loop or FUSE mount the file somewhere

    Then when you want to clear the data: unmount, format, and remount the file.

    Added benefit of testing no more disk space conditions easily.

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

      That’s an interesting way of doing it. I do not try to avoid rm completely, just want to make sure the construct doesn’t have any obvious issues. Your suggestion is a bit too much for the few test files I wanted to create and delete.

  • moonpiedumplings@programming.dev
    link
    fedilink
    English
    arrow-up
    1
    arrow-down
    1
    ·
    2 months 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
      ·
      2 months 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.