this post was submitted on 20 Sep 2026
21 points (95.7% liked)

Programming

28800 readers
143 users here now

Welcome to the main community in programming.dev! Feel free to post anything relating to programming here!

Cross posting is strongly encouraged in the instance. If you feel your post or another person's post makes sense in another community cross post into it.

Hope you enjoy the instance!

Rules

Rules

  • Follow the programming.dev instance rules
  • Keep content related to programming in some way
  • If you're posting long videos try to add in some form of tldr for those who don't want to watch videos

Wormhole

Follow the wormhole through a path of communities !webdev@programming.dev



founded 3 years ago
MODERATORS
 

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.

top 50 comments
sorted by: hot top controversial new old
[–] eleijeep@piefed.social 8 points 3 weeks ago (1 children)

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.ml 0 points 3 weeks ago (1 children)

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 3 points 3 weeks ago (1 children)

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.ml 0 points 3 weeks ago* (last edited 3 weeks ago) (1 children)

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 2 points 3 weeks ago* (last edited 3 weeks 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 :)

[–] tal@lemmy.today 7 points 3 weeks ago

While I'm all for being careful with recursive removals, GNU rm won't clobber specifically / without --no-preserve-root.

[–] MonkderVierte@lemmy.zip 4 points 3 weeks ago (1 children)

cd-ing to a directory first, is way more risky than just giving rm -r the path, imo. And the whole bunch of rm file, go one up, rmdir directory, is just peace-of-mind that actually makes the trap more fragile.

[–] thingsiplay@lemmy.ml 1 points 3 weeks ago (1 children)

Why do you think so? Changing directory can only be done, if it exists, otherwise cd will error out and rm command not run. After the command the temporary directory is empty and cd can go up one dir, to use rmdir on an empty directory. Which of the steps are fragile and why?

[–] MonkderVierte@lemmy.zip 3 points 3 weeks ago* (last edited 3 weeks ago) (9 children)

I mean, multiple commands and changing directories vs. one command. Trust me here, that's more fragile.
And the advantage of rmdir over rm -r is, that it only deletes empty directories, is safer in some usecases. But this is moot, if you delete it's content anyway.

And if you do have multiple commands in a trap, i recommend exporting them to a function; easier to grok.

load more comments (9 replies)
[–] mantricx@lemmy.world 3 points 3 weeks ago (1 children)

You could check if TEMPDIR is set and exists as a directory but LGTM.

[–] thingsiplay@lemmy.ml 2 points 3 weeks ago (2 children)

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

[–] hendrik@palaver.p3x.de 2 points 3 weeks ago (1 children)

I think so as well. I'd run it like this.

[–] thingsiplay@lemmy.ml 1 points 3 weeks 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 0 points 3 weeks ago (1 children)

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

load more comments (1 replies)
[–] MadhuGururajan@programming.dev 1 points 3 weeks 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

[–] A_norny_mousse@piefed.zip 1 points 3 weeks ago (1 children)

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

Do you even need to use -f with rm?

[–] thingsiplay@lemmy.ml 1 points 3 weeks ago (1 children)

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.

[–] A_norny_mousse@piefed.zip 1 points 3 weeks ago* (last edited 3 weeks ago) (1 children)

cd can only enter a directory that exists anyway

OK now imagine that $TEMPDIR is not set (something @eleijeep@piefed.social also alludes to in their top-level comment).

cd "$TEMPDIR/tests" now becomes cd "/tests"

[ -d "" ] always fails.

It depends if I create subdirectories in an uncontrolled manner.

This has little to do with the -f option. I wasn't refering to the -r option. Did you even read man rm?

[–] thingsiplay@lemmy.ml 1 points 3 weeks ago (1 children)

Did you even read man rm

That was an unnecessary snarky comment.

[–] A_norny_mousse@piefed.zip 1 points 3 weeks ago* (last edited 3 weeks ago)

Only someone who doesn't read man pages would reply like that.

OK let me rephrase: do you even know what the -f option is there for?

[–] coolie4@lemmy.world 1 points 3 weeks ago (1 children)

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.ml 1 points 3 weeks 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.

[–] FizzyOrange@programming.dev 1 points 3 weeks ago (1 children)

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.ml 1 points 3 weeks ago (1 children)

./$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.

[–] FizzyOrange@programming.dev 2 points 3 weeks ago

Ah good point. Guess you'll have to explicitly check.

[–] HubertManne@piefed.social 1 points 3 weeks ago (1 children)

Honestly I don't think I have ever use rm -rf in a script.

[–] thingsiplay@lemmy.ml 1 points 3 weeks ago (1 children)

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 2 points 3 weeks 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.

[–] TehPers@beehaw.org 1 points 3 weeks ago* (last edited 3 weeks ago) (1 children)

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.ml 0 points 3 weeks ago (1 children)

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 1 points 3 weeks ago (1 children)

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.ml 0 points 3 weeks ago (1 children)

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 0 points 3 weeks ago (1 children)

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

load more comments (1 replies)
[–] IanTwenty@piefed.social 1 points 3 weeks ago (1 children)

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.ml 1 points 3 weeks ago* (last edited 3 weeks ago) (1 children)

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 2 points 3 weeks ago (1 children)

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.ml 2 points 3 weeks 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.

[–] arthropod_shift@programming.dev 1 points 3 weeks ago* (last edited 3 weeks ago) (4 children)

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.

load more comments (4 replies)
[–] ThanksForAllTheFish@sh.itjust.works 1 points 3 weeks ago* (last edited 3 weeks ago) (1 children)

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.ml 1 points 3 weeks 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.

[–] moonpiedumplings@programming.dev 0 points 3 weeks ago* (last edited 3 weeks ago) (1 children)

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.ml 0 points 3 weeks 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.

load more comments
view more: next ›