Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moved ~/.local/share/steam. Ran steam. It deleted everything on system owned by user. #3671

Closed
keyvin opened this issue Jan 14, 2015 · 266 comments
Labels

Comments

@keyvin
Copy link

keyvin commented Jan 14, 2015

Edit: Please stop posting stupid image memes or unhelpful messages. This interferes with Valve's ability to sift through the noise and see if anyone can figure out what triggers it.

This may not be a common problem because I change all sorts of configuration about my system. The script in question does something in a really, really stupid way, but it probably doesn't trigger the fail scenario for every system because...

Original Bug:
I am not sure what happened. I moved the folder in the title to a drive mounted under /media/user/BLAH and symlinked /home/user/.local/steam to the new location.

I launched steam. It did not launch, it offered to let me browse, and still could not find it when I pointed to the new location. Steam crashed. I restarted it.

It re-installed itself and everything looked great. Until I looked and saw that steam had apparently deleted everything owned by my user recursively from the root directory. Including my 3tb external drive I back everything up to that was mounted under /media.

Everything important, for the most part, was in the cloud. It is a huge hassle, but it is not a disaster. If there is the chance that moving your steam folder can result in recursively deleting everything in the directory tree you should probably just throw up an error instead of trying to point to other stuff. Or you know, allow the user to pick an install directory initially like on windows.

My system is ubuntu 14.04, and the drive I moved it to was ntfs if its worth anything.

@doofy
Copy link

doofy commented Jan 15, 2015

I am impressed how calm you stay about this. This is terrible. I just lost my home directory. All i did was start steam.sh with STEAM_DEBUG=1. I will investigate this and report back.

edit1: I suspect steam.sh got some bugs(does not check own variables) and when it tried to do scary things it crapped himself.
Line 468: rm -rf "$STEAMROOT/"*

edit2: It gets better. Seems on windows Steam is overeager too! https://support.steampowered.com/kb_article.php?ref=9609-OBMP-2526 (The warning part is interesting. Because everybody reads this before uninstalling...)

@Tele42
Copy link

Tele42 commented Jan 15, 2015

I agree, that line minimally requires an exists and not null check for $STEAMROOT

@keyvin
Copy link
Author

keyvin commented Jan 15, 2015

#scary!

As an ex programmer, that really makes me chuckle. Can I at least get an apology from whoever committed that comment without adding a fix?

@CamilleScholtz
Copy link

This also happened to me a few weeks ago, my entire home was deleted by the steam.sh script.

@pythoneer
Copy link

introduced here Sep 10, 2013 https://github.com/indrora/steam_latest/commit/21cc14158c171f5912b04b83abf41205eb804b31 line 359

rm -rf "$STEAMROOT/"* could be evaluated as rm -rf "/"* if $STEAMROOT is empty

but what exactly caused this? i've symlinked ~/.local/share/steam to, so i am a bit afraid to start steam :/

@TcM1911
Copy link

TcM1911 commented Jan 15, 2015

pythoneer,

I believe the issue starts on line 19:

# 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)"
STEAMDATA="$STEAMROOT"

This probably returns as empty which mean: rm -rf "$STEAMROOT/"* is the same ass rm -rf "/"*.

@pythoneer
Copy link

TcM1911,

that's my guess, too.

@minio
Copy link

minio commented Jan 15, 2015

@keyvin @d00fy :
Line proceeded by # Scary! comment is in reset_steam() function, which is executed if and only if steam.sh is invoked with --reset as first argument.

Did any of you deliberately invoked that script with that option? If yes, why did you do it? What were you trying to achieve?

Removing user data is obviously wrong, no doubt about it. But if this happens only when user requests certain action, scope of that issue is somewhat limited.

@jthill
Copy link

jthill commented Jan 15, 2015

Yeah, they kinda need a readlink in there.

STEAMROOT=$(readlink -nf "${0%/*}")

@jthill
Copy link

jthill commented Jan 15, 2015

@minio Not "only if". reset_steam is also invoked by removing your .steam directory, since that sets INITIAL_LAUNCH

@soren121
Copy link

@minio A script accidentally running rm -rf /* is unacceptable in any scenario.

@gtmanfred
Copy link

it is like bumblebee all over again!
MrMEEE/bumblebee-Old-and-abbandoned#123

@Plaque-fcc
Copy link

I encountered Steam behaviour like with «--reset» for several times:
when I added «--reset» AND when I didn't. So — not «only if», can
confirm this, even having not deleted ~/.steam/ dir, too.

@prometheanfire
Copy link

wonder what the code path is to hit that rm without --reset

@indrora
Copy link

indrora commented Jan 15, 2015

Can confirm; I have Steam bounded in an SELinux context ("Steam") and SELinux spits out:

Context violation: process /home/indrora/.local/share/Steam/ubuntu12_32/steam is only allowed in context steam_context, attempted to remove /boot/efi/grub/efistub

Ooops. I'll write a patch and PR it 🍺

@indrora
Copy link

indrora commented Jan 15, 2015

@johnv-valve johnv-valve self-assigned this Jan 15, 2015
@johnv-valve
Copy link
Contributor

Does anybody have reliable repro steps for this? I can easily add the checks for STEAMROOT being empty, but I would also like to fix the root cause if possible.

@rcxdude
Copy link

rcxdude commented Jan 15, 2015

It will definitely fail if you run steam.sh as bash steam.sh. I don't know if that's the cause in this case. In terms of root cause, I would say you should use set -e, set -u, and similar options in order to make the script less likely to silently ignore errors.

@ayust
Copy link

ayust commented Jan 15, 2015

Using ${STEAMROOT:?} instead of $STEAMROOT would have helped, too.

(For those not familiar, ${FOO:?} is identical to $FOO except that it errors out automatically if $FOO is empty or unset.)

@ju2wheels
Copy link

Which is the same thing that would have happened had the unnecessary '/*' not been there anyway, it would have errored out. Its not necessary because the rm was set to recursive already...

if [ ! -z "${STEAMROOT}" ]; then
   rm -rf "${STEAMROOT}"
fi

@rcxdude
Copy link

rcxdude commented Jan 16, 2015

Here is a patch which enables set -e, set -u, and a few others, and then fixes up all the places where undefined variables are expected to be (as far as I can tell): https://gist.github.com/rcxdude/1f6257e0a965147a462c

@mablae
Copy link

mablae commented Jan 16, 2015

@rcxdude Come on. We are on github here. Do that in a PR please! Do not post whole patch files into issues... 👎

@dannyfallon
Copy link

@mablae There is no code on this repo, there is nothing to send in a PR against. A gist wouldn't have gone astray though 😄

@stefan-pdx
Copy link

@rcxdude: Please link to a Gist.

@sdt16
Copy link

sdt16 commented Jan 16, 2015

@mablae How about instead of being a jerk, you link @rcxdude to some documentation.

@Tele42
Copy link

Tele42 commented Jan 16, 2015

@johnv-valve regardless of tracking down the cause of this, this rm line must be protected from future accidental gremlins due to the severity of the fail scenario.

@mablae
Copy link

mablae commented Jan 16, 2015

@dannyfallon Oh, sorry then...
@sdt16 I am sorry. Thought the code was on this repo ... Didn't wanted to be a "jerk" :)

@Wapaca
Copy link

Wapaca commented Jan 16, 2015

Does it happen only if you move ~/.local/share/steam ? #scary! :s

@joshenders
Copy link

The idiomatic way to do this in Bash is to use default variables as in ,"${var:-/sane/path}" or "${var:?}" as was already mentioned. While using set -u or similar could have prevented this mistake, it's lazy and considered bad style.

@reverofevil
Copy link

@ju2wheels But a Half Life 3 talk didn't even have a chance to start!

@user15177
Copy link

Has this problem been fixed? I really want to play the metro series, but this scares the crap out of me.

@ghost
Copy link

ghost commented Nov 28, 2017

@user15177 Set up a chroot for Steam and run it from there so it can do no harm to your actual system.

@alphapapa
Copy link

@craig-sanders Since you posted some cleaned-up code from the script and some good Bash scripting advice, a note: Always use the [[ ]] test syntax in Bash-specific scripts. It's safer and cleaner in all cases.

ethomson added a commit to ethomson/azure-pipelines-agent that referenced this issue Feb 19, 2019
When expanding variables to pass to `rm`, make sure that the path
variable is set and fail if it is not.  This prevents an `rm` from
accidentally expanding to `rm ""/*` when the variable is unset:
ValveSoftware/steam-for-linux#3671

Using `${FOO:?}` syntax will fail when `FOO` is unset.
TingluoHuang pushed a commit to microsoft/azure-pipelines-agent that referenced this issue Feb 19, 2019
* dev.sh: use dash instead of slash for msbuild

Instead of switching between windows and non-windows to determine how to
handle slashes for msbuild, use dashes instead of slashes to simplify
the calling.

* dev.sh: stop on errors

Stop on errors, instead of continuing.  This prevents us from failing to
move through the directory space with `cd` / `pushd` / `popd` but still
running commands.  This is particularly dangerous when running commands
like `rm`.

* dev.sh: quote all filepaths

Since directories may have a space in them, quote them to treat them as
a single entity instead of wordsplitting on a space.

Otherwise, if `FOO="a b c"` then `rm -rf $FOO` will remove files or
folders named `a`, b`, and `c` instead of removing the single entity
named `a b c`.

* dev.sh: remove files carefully

When expanding variables to pass to `rm`, make sure that the path
variable is set and fail if it is not.  This prevents an `rm` from
accidentally expanding to `rm ""/*` when the variable is unset:
ValveSoftware/steam-for-linux#3671

Using `${FOO:?}` syntax will fail when `FOO` is unset.

* dev.sh: quote the `$` in `$LastExitCode`

`$LastExitCode` is not a bash variable; to pass that string along to
PowerShell, it needs to be quoted.

* dev.sh: use $(cmd) syntax instead of backticks

The $(cmd) execution syntax is preferred over the legacy backtick
syntax.

* dev.sh: quote variables

* externals.sh: quote variables to cope with spaces

Quote the variables for the directories so that we can properly work
with directories with spaces in their names.
@aviwad
Copy link

aviwad commented Mar 17, 2019

Is this fixed? xD

@aviwad
Copy link

aviwad commented Mar 17, 2019

whoa what the heck does the unassigned message mean

@aaronfranke
Copy link

@aviwad It's just something GitHub does automatically when the assigned person doesn't actually do anything about the issue and someone comments on it again.

@nicklleite
Copy link

Its still happening. It deleted almost everything on my Ubuntu.

@kisak-valve
Copy link
Member

Hello @nicklleite, the investigation of the issue in this issue report concluded several years ago. Please open a new issue report.

@ghost

This comment has been minimized.

@antdude
Copy link

antdude commented Jun 2, 2021

?

@ValveSoftware ValveSoftware locked as resolved and limited conversation to collaborators Jun 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests