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.
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
/tmpfor 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.
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 inrm -rf "${TEMPDIR}"vsrm -rf "${TEMPDIR}/tests". Because I’m not worried about whatmktempgives 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
TEMPDIRcontent ifmktempcreated it correctly. I’m more worried about the variable being altered and invalid at later point in the script. I would rather leavemktempcreate 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.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.
Ah I absolutely misunderstood you. The PID trick is actually clever!
But I would still not solely want to rely on a
$TEMPDIRvariable 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.
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.
Some solutions:
set -e TEMPDIR=$(mktemp -d) # script exits if mktemp returns erroror
TEMPDIR=$(mktemp -d || echo "/invalid") # TEMPDIR gets the value "/invalid" if mktemp failsor
TEMPDIR=$(mktemp -d) TEMPDIR=${TEMPDIR:-"/invalid"} # TEMPDIR gets the value "/invalid" if mktemp returns an empty valueor
TEMPDIR=$(mktemp -d) rm -rf ${TEMPDIR:-"/invalid"} # rm is passed "/invalid" if TEMPDIR is emptyMy personal preference is the last one. Any time you call
rm -rfyou provide a default value to variables to ensure that if they are empty they get some other value instead.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 -eoption, as I do not want he entire script to exit on error. So instead I can use theexitcommand 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 bycd "${TEMPDIR}/tests" || exit, so the script never continues without a successfulmktempdirectory.TEMPDIR=${TEMPDIR:-"/invalid"}I always forget that Bash has default values for variables!
mktempactually 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$TEMPDIRby accident (in example to something empty). So assigning a default value aftermktempwill 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.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.
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.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/nullwhich 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 :)
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.I think so as well. I’d run it like this.
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 withrm -rf tests, which is empty at that point.So you see even if it looks perfectly valid…
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.
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.
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.
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.
I think you should start the last line with
[ -d "$TEMPDIR" ] && cd ....Do you even need to use
-fwithrm?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.
Honestly I don’t think I have ever use rm -rf in a script.
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.
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.



