[akamaka]

Here's the offending shell script code:

  # figure out the absolute path to the script being run a bit
  # non-obvious, the ${0%/*} pulls the path out of $0, cd's into the
  # specified directory, then uses $PWD to figure out where that
  # directory lives - and all this in a subshell, so we don't affect
  # $PWD
  STEAMROOT="$(cd "${0%/*}" && echo $PWD)"
  [...]
  # Scary!
  rm -rf "$STEAMROOT/"*

The programmer knew the danger and did nothing but write the "Scary!" comment. Sad, but all-too-familiar.

[thaumaturgy]

Yikes. That should never have passed a code review. I know mistakes happen, I often defend screw-ups, but anything in a shell script that has an "rm" command should automatically get the squinty eyes.

[whoisthemachine]

Also, someone should have been asking "Why is that scary?" - which should have lead to a discussion on alternatives.

[ghuntley]

Offending commit @ https://github.com/lrusak/steam_latest/commit/21cc14158c171f...

[gcp]

Actually, that repo is just a user who extracted the Steam .debs into a github repo, the comments are completely misdirected.

[zamalek]

The reaction images/memes are priceless.

[EC1]

I thought I was a bad programmer until I saw this thread. How did this make it to prod?

[greenknight]

Tight deadlines, limited resources (programmers), limited budget, limited QA

[deeviant]

I worked at a solar SCADA company that rolled there own APT packages.

In the pre and post install deb package scripts there was all kinds of crazy shit, like upgrading grub to grub 2 and manually messing with boot sectors. All this stuff in packages innocuously named modbus_driver.deb or what have you, and all in absolutely the most archaic bash syntax possible. I did suggest, strongly to jail all application binarys that we twiddle around with, with something like chroot, but was rebuffed.

Eventually somebody mixed a rm -rf /bin/* with rm -rf / bin/*, and the rest is history. They bricked about 100 embedded PC's, all in remote locations, all powering powerplant SCADA systems that did stuff like connect to CalISO for grid management or collect billing information. It cost hundreds of thousand of dollars to fix.

[userbinator]

Would this have stopped things getting deleted?

    if [ -z "$STEAMROOT" ]
       # something isnt right...

[colanderman]

Not if the previous line was:

    STEAMROOT=$SOME_OTHER_UNSET_VARIABLE/

"rm -r " is a code smell, as much as "cc -o myprog .c" is. You should always know what files make up your system, and track them in a MANIFEST file. There's rarely a good reason to use wildcards when a program is dealing with its own files.

    xargs rm -df -- < MANIFEST

fixes this.

[__david__]

That looks like it works until your MANIFEST file ends up with a space character in one of the file names.

For GNU xargs I like adding -d\\n which handles everything except files with an embedded newline. Those are much rarer than files with a space, though.

Sadly, OS X xargs (probably BSD based) doesn't have that option, so I have an alias to do the same thing:

    alias xargsn="tr '\n' '\0' | xargs -0"

[colanderman]

Good catch.

[jblow]

Steam doesn't deal with its own files. It deals mainly with random games that are creating tons of random files.

[mercurial]

Doesn't it have an API? It could mandate that "random files" should only be created and deleted via the API, and update the manifest accordingly. Put the game in a read-only folder to make sure it happens.

[vertex-four]

Steam actually sells a number of games which haven't been modified for use with Steam at all - no DRM integration, no achievements. Further to that, it sells games which use closed engines that are never going to be modified to use Steam's APIs to do things.

[hobs]

Or worse, basically just glorified installers for games for windows live, or ubisoft's giant portal thing that you have to then run simultaneously (or in the right order) to get to the game.

Its definitely a rube goldberg machine in action.

I feel like the idea of sandboxing its progeny is going to need to look like docker or some sort of container where it appears to be a standard OS (since games use a lot of low level hacks) but is actually partitioned from the rest of the system.

[falcolas]

   rm --preserve-root

isn't a bad thing to have either. That way, even if you do screw up, you won't be able to run rm against '/', even with '-f'.

It's one of the top aliases in my .bash_aliases file.

[colanderman]

Agreed. Thankfully this is the default on many systems (I'm guessing recent coreutils).

Unfortunately dumb lines like "rm -rf $HOME/$STEAMDIR" still get through unscathed.

[philh]

I think that

    rm --preserve-root -rf /*

doesn't save you.

[colanderman]

You are quite correct.

[Zikes]

What if you somehow wound up with a single line of / in the MANIFEST, though?

[colanderman]

"rm -df /" does nothing. "rm -df" does not remove non-empty directories.

[Zikes]

That's awesome, I didn't catch the s/r/d/.

[colanderman]

That's because I ninja'd the -d in after ;) also just ninja'd in a -- to prevent stray options.

Note also that putting a "*" in the MANIFEST doesn't do anything either, as neither xargs nor rm expands wildcards (only bash does).

[Hello71]

    rm: cannot remove ‘/’: Is a directory

[8_hours_ago]

Fyi: the -d option is specific to the BSD implementation of rm, it's not available in the GNU coreutils rm.

[colanderman]

It's in my coreutils (8.22). Maybe it's recently added?

[8_hours_ago]

It looks like it was added in version 8.19. Probably about time to update my Ubuntu box...

http://savannah.gnu.org/forum/forum.php?forum_id=7342

[Alupis]

> Would this have stopped things getting deleted?

No. "${STEAMROOT}" will contain something when the "rm" command runs. It just might not be what's expected.

> STEAMROOT="$(cd "${0%/*}" && echo $PWD)"

[deeviant]

I worked at a solar SCADA company that rolled there own APT packages.

In the pre and post install deb package scripts there was all kinds of crazy shit, like upgrading grub to grub 2 and manually messing with boot sectors. All this stuff in packages innocuously named modbus_driver.deb or what have you, and all in absolutely the most archaic bash syntax possible.

Eventually somebody mixed a rm -rf /bin/* with rm -rf / bin/*, and the rest is history. They bricked about 100 embedded PC's, all in remote locations, all powering powerplant SCADA systems that did stuff like connect to CalISO for grid management or collect billing information. It cost hundreds of thousand of dollars to fix.

[Qantourisc]

Whenever I write something scary like that, I usually wrap it in a function, double checking if it's really the folder you wish to delete ... Often even adding a user-verification if possible.

[iamtew]

Since the script is written in bash they could also have used $BASH_SOURCE, which will always point to the correct path of the script being executed, so you can do something like this:

    SCRIPT="$BASH_SOURCE"
    SCRIPT_DIR="$(dirname "$BASH_SOURCE")"

[enneff]

Gotta question why they used -f.

[unwind]

That's normal, but why the trailing slash?! That's just pointless, and almost looks like an explicit deathtrap.

Without the slash, an empty variable would result in a command line of "rf -rf" which would simply fail due to the missing argument.

There is absolutely no need for having a trailing slash, it's not as if "rf -rf foo" and "rm -rf foo/" can ever mean two different things, there can be only one "foo" in the file system after all.

Very interesting way of introducing an epic fail with a single character, that really looks harmless.

[riquito]

> it's not as if "rf -rf foo" and "rm -rf foo/" can ever mean two different things

That's true, but the original code was akin "rm -rf foo/*" and that's different, since it removes the content of the directory while preserving it.

[teddyh]

I have observed that ,any people feel that directories “need” to have a slash appended to them. They are afraid of ever doing, e.g. “cd /foo/bar” and will always do “cd /foo/bar/”. I”m guessing they feel like it would be some sort of type error, like treating a directory like a file or something.

This behavior is especially common with regards to URLs; there are many flamewars about whether URLs “need” a trailing slash or not.

[pixelbeat]

The trailing /* is to delete the directory _contents_ rather than the dir itself. It would be safer to receate the dir as then you can possibly hit some of the inbuilt rm protections

[giggles_giggles]

So the script wouldn't pause and ask for user input during normal operation.

[enneff]

But why would the script prompt for user input unless something was awry? Presumably they control the contents of $STEAMROOT, so I don't see why rm -r should prompt unless it's about to do the wrong thing.

[Alupis]

most systems have "alias rm='rm -i'" by default, so it would prompt on every single file regardless of ownership, etc.

[enneff]

"most systems"? Not on Debian or Ubuntu, and that's most of them.

[smtddr]

fwiw, making those "-i" aliases to rm, mv and cp are one of the first things I do on any new Linux machine I'm on.

I don't understand how anyone works in the very unforgiving-of-accidental-delete *nix world without those aliases.

[0942v8653]

or Arch Linux, or OS X, or Cygwin, or …

… I don't think anyone does that.

[Alupis]

> Not on Debian or Ubuntu, and that's most of them

Perhaps I should have said "sane systems" ;)

Fedora ships with that alias by default FWIW.

[dezgeg]

Shells (at least bash) don't evaluate aliases when running non-interactively.

[firethief]

Anyone considering putting customization like aliases where they'll execute even for non-interactive shells should go home and rethink their life. Thankfully, I've never encountered such a system.

[frou_dh]

If all one is in the habit of using with a particular command is some single letter flags glommed together like that then it's possible to forget that they're actually separate flags.

[FranOntanaya]

Probably to delete subfolders.