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...
- Use a subdirectory, so the variable is not used by itself. So we have to use
${TEMPDIR}/testseach time instead just${TEMPDIR}. - 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 thatrm -rfis only executed if the temporary directory even exist and the variable is not resolved to empty. - 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.
You could do
./$TEMPDIRor 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.
./$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.
Ah good point. Guess you’ll have to explicitly check.
You could check if TEMPDIR is set and exists as a directory but LGTM.
Isn’t
cdwith&&essentially doing that? Chain only runs, if TEMPDIR exists as a directory.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
Oh good catch. I meant to use
mkdir -p, which is basically what-fto force in other commands is. It is actually not needed in this case anyway. I will also add --verbose to it.
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.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.
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?
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.
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
However, I am very, very afraid of doing
rm -rfin 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
$TEMPDIRchanging a reasonable case to handle? Can you change your script design to make this impossible instead? (Like replacing asourcewith 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 renameTEMPDIRto something likeTEST_ROOTif 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.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.
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->rmdirsnippet covers that’s missed byset -u->rm -rf?I generally dislike using the
setmethods 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?
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 -ubecause 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.
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
TEMPDIRdirectory some other way during the script, and potentially even recreatestestsafter 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
$TEMPDIRexisting before a simplerm -rf "$TEMPDIR".However, I am very, very afraid of doing
rm -rfin 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 -rfthe directory. No need to overcomplicate it.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
$TEMPDIRis never changed from the perspective of the script. Because its not exposed to subprocesses and the name is totally random bymktemp. 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
$TEMPDIRby accident in a loop, instead reading from it. So at the time of execution ofrm -rf, there is a chance that$TEMPDIRcould potentially point to a different directory in example.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
$TEMPDIRby accident in a loop, instead reading from it. So at the time of execution ofrm -rf, there is a chance that$TEMPDIRcould potentially point to a different directory in example.Create another variable for use in the script, and only use
$TEMPDIRfor 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.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?
mktempmakes sure its 100% random. And any process on the system can’t just read the variable out.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).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 runningrm -rfcommand. 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.
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.
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.
One workaround is that you could use the
trash-clicommand, 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.
I actually have
trash-cliinstalled 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.


