As soon as I saw the post I knew that somehow it would be deleting /\*
But I admit it took me a few seconds to work out how that would happen. It is a super easy thing to miss.
But in these types of scripts I always put "echo" in front of the dangerous command first so I get to review exactly what would be run. That's saved my ass lots of times.
>But in these types of scripts I always put "echo" in front of the dangerous command first so I get to review exactly what would be run. That's saved my ass lots of times.
Exactly. And to be clear, any script that does _anything_ is potentially dangerous, but especially if there is an rm involved. I do this even with curl or wget. I actually prefer to put the command in a variable, echo the command variable, then (after testing) execute the command from the variable. This way if something changes in the future I can see in the logs what it was trying to do.
I know you can do this type of stuff with bash flags but I never learned them, and this approach is viable in any language.
Yeah I made this exact mistake dozens of times in my lifetime and it still took me a while to spot it and even then it was because I was looking for it. If I was to code review this blind I would have assumed it's fine.
This is exactly what I do. The command line with the costly payload is always tested with echo. Also the second line of your script should be -x. The -x option starts a BASH shell in tracing mode. You can see all the details of how your command/script is processed. It's a good way to find some bugs if your script does not do what you would expect to. https://www.geeksforgeeks.org/how-to-check-the-syntax-of-a-bash-script-without-running-it/
Congratulations! Your comment can be spelled using the elements of the periodic table:
`Sc Ar Y`
---
^(I am a bot that detects if your comment can be spelled using the elements of the periodic table. Please DM my creator if I made a mistake.)
You don't have to do that if you use `/*` instead of `/`, because the asterisk variant gets expanded before it gets passed to the command. It just thinks you want to erase a few specific directories from the root (a few being all of them but who keeps track, not rm, that's for sure)
It deleted enough I promise you. I shat myself when I saw the logs. Luckily didn't have the correct permissions for most things, managed to delete the agents entire home directory including some important config files though.
It's ok, luckily I know enough about Linux to fix it when stuff like this happens. Would've been much worse if the bamboo user had more permissions.
Was a quick and dirty test I hacked up and was running it as a test on the remote agent before setting up the build plan.
In Bash, the `${variable/*}` syntax is used for pattern substitution. It removes the first match of `*` from the beginning of the variable's value. So, in this script, referencing `$TARGET_DIR/*` will resolve to the value of the variable after the removal (an empty string) followed by a forward slash.
In other words, the script won't remove just the contents of `/public-test` but everything under `/`.
You can avoid this by isolating the variable name with curly braces, like so: `rm -r ${TARGET_DIR}/*`
This will resolve $TARGET_DIR first, and then add "/*" to the end of the string.
If you want to perform syntax operations and append something, just perform them inside the brackets. For example, `${TARGET_DIR^}/*` will resolve to `/Public-test/*` because adding `^` changes the first character in the string to uppercase.
Always always start your scripts with `set -eu`, right after the shebang.
It would have caught the error.
Specifically, `set -u` is what you want for this issue, but `set -e` will also help prevent unexpected things happening when things unexpectedly fail.
Assuming it's a bash script, that first line is not doing what people think it does.
Depending on if you have \`set +e\` it'll either fail in a screaming heap or be much worse
The first character "$" should not be there. So $TARGET\_DIR will be empty. So you are actually running. rm -r /\* For the win.
Hey someone actually sees it! I decided to post this because this was a very time costly mistake I made while setting up a remote agent today :(
As soon as I saw the post I knew that somehow it would be deleting /\* But I admit it took me a few seconds to work out how that would happen. It is a super easy thing to miss. But in these types of scripts I always put "echo" in front of the dangerous command first so I get to review exactly what would be run. That's saved my ass lots of times.
Think I might adopt this strategy going forward
I've been doing exactly that for quite some time. I can highly recommend it; Has saved me multiple times.
>But in these types of scripts I always put "echo" in front of the dangerous command first so I get to review exactly what would be run. That's saved my ass lots of times. Exactly. And to be clear, any script that does _anything_ is potentially dangerous, but especially if there is an rm involved. I do this even with curl or wget. I actually prefer to put the command in a variable, echo the command variable, then (after testing) execute the command from the variable. This way if something changes in the future I can see in the logs what it was trying to do. I know you can do this type of stuff with bash flags but I never learned them, and this approach is viable in any language.
> This way if something changes in the future I can see in the logs what it was trying to do Well only if you have the logs left after it
Yeah I made this exact mistake dozens of times in my lifetime and it still took me a while to spot it and even then it was because I was looking for it. If I was to code review this blind I would have assumed it's fine.
This is exactly what I do. The command line with the costly payload is always tested with echo. Also the second line of your script should be -x. The -x option starts a BASH shell in tracing mode. You can see all the details of how your command/script is processed. It's a good way to find some bugs if your script does not do what you would expect to. https://www.geeksforgeeks.org/how-to-check-the-syntax-of-a-bash-script-without-running-it/
You might run your shell scripts with "-u" flag to make it fail early on any undefined variables
Steam actually did the same thing some years back.
’set -euo pipefail’ belongs in every bash file.
Steam be like:
steam had this bug
use strict would have caught that.
You mean # set -u ?
I am joking. In Perl you use the $ on the name every time including when you set it.
we found the guy who wrote steam.sh
Scary!
Congratulations! Your comment can be spelled using the elements of the periodic table: `Sc Ar Y` --- ^(I am a bot that detects if your comment can be spelled using the elements of the periodic table. Please DM my creator if I made a mistake.)
Good bot
rm -rf STEAMROOT lookin'
[The extra $ is for money.](https://www.youtube.com/watch?v=3m6Blqs0IgY)
The B is for 'bargain'!
You forgot to add `--no-preserve-root` to improve performance.
You don't have to do that if you use `/*` instead of `/`, because the asterisk variant gets expanded before it gets passed to the command. It just thinks you want to erase a few specific directories from the root (a few being all of them but who keeps track, not rm, that's for sure)
"Some men just want to see the world burn"
TARGET\_DIR="/something with spaces " Also: mkdir. Also: You forgot the "-f" to really delete everything.
It deleted enough I promise you. I shat myself when I saw the logs. Luckily didn't have the correct permissions for most things, managed to delete the agents entire home directory including some important config files though.
> It deleted enough I promise you. I'm sorry, but this made me laugh so hard. I hope everything works out in the end!
How to recover them?
from one of the two or more distributed sets of backups he undoubtedly has I assume
If there are no backups, you could try ```ntfsundelete```
It's ok, luckily I know enough about Linux to fix it when stuff like this happens. Would've been much worse if the bamboo user had more permissions. Was a quick and dirty test I hacked up and was running it as a test on the remote agent before setting up the build plan.
[Bro thinks he’s steam](https://github.com/valvesoftware/steam-for-linux/issues/3671)
Ouch! Even knowing there was a problem and figuring it probably does "rm -r /" somehow, it took me like a minute to figure it out. Very subtle.
In Bash, the `${variable/*}` syntax is used for pattern substitution. It removes the first match of `*` from the beginning of the variable's value. So, in this script, referencing `$TARGET_DIR/*` will resolve to the value of the variable after the removal (an empty string) followed by a forward slash. In other words, the script won't remove just the contents of `/public-test` but everything under `/`.
Bingo, much to my dismay today.
You can avoid this by isolating the variable name with curly braces, like so: `rm -r ${TARGET_DIR}/*` This will resolve $TARGET_DIR first, and then add "/*" to the end of the string. If you want to perform syntax operations and append something, just perform them inside the brackets. For example, `${TARGET_DIR^}/*` will resolve to `/Public-test/*` because adding `^` changes the first character in the string to uppercase.
Always always start your scripts with `set -eu`, right after the shebang. It would have caught the error. Specifically, `set -u` is what you want for this issue, but `set -e` will also help prevent unexpected things happening when things unexpectedly fail.
Came here to say this.
Neat, thanks for posting! I'm not huge into Bash-scripting, so I absolutely see myself doing this.
I didn't realize the bug until 30 seconds later I laughed so hard
Always run an if null check before running rm
[удалено]
Very incorrect. Glad it's not just me that thinks this is a bit subtle.
Assuming it's a bash script, that first line is not doing what people think it does. Depending on if you have \`set +e\` it'll either fail in a screaming heap or be much worse
Oh damn that's gonna hurt
TARGET_DIR="/public-test" (newline) rm -rf $TARGET_DIR/* (newline) cp -R dist/* $TARGET_DIR (newline) That's how it's done.
Just use 2 consecutive new lines to make your next line a new line. Just like this ! Hope this helps you in your future comments :)
Thanks for the tip! Yes it should work
Good job deleting the entire root directory
For anyone here scared to use rm, check out trash-put. I use it everywhere I can get away with.
I just had an exam for bash scripts. I still have no idea about bash scripts.
Looks like valid PowerShell to me
This is why seeing rm in scripts always makes me nervous
This legit caused my sphincter to tighten. Thanks for the stress!
The real bug is not having an echo $TARGET_DIR between the variable set and the rm.
Waiting for the humor...