Unvanquished/updater

prevent people "accidentally" run updater as root

bmorel opened this issue · 6 comments

I've seen on IRC that some people run the updater as root, and then chmod foo:bar $GAMEDIR. Running stuff as root can be handful, but also reserved to people which know what and why they're doing it, so, on linux and bsd systems, I would check against geteuid() == 0 (or getuid, in case someone gave the updater the setuid bit, unlikely, but code is the same and this feels safer to me?)

So the case we especially need to avoid is when the user runs as root, uses the default install location, and ~/.local/share/unvanquished does not already exist. This results in the homepath being owned by root, so the game is not usable.

I think what I'll do is, when the user clicks "start download", to test if the install location is inside $HOME (a condition more general and future-proof than looking specifically for the homepath). If so, tell the user to restart as non-root and close the program.

It would also be nice to ensure that Unix permission bits include world-readable and -executable if an install as root is done somewhere else (I'm hoping this will just be inherited from the parent directory and we wouldn't have to do anything).

Can we just yell at people running as root and complain or at least make them work a little harder asking if this is something they want to do?

if (getuid() == 0) { // complain }

to test if the install location is inside $HOME

This is not a good idea. The updater follows the XDG specification allowing anyone to replace ~/.local/share/ with the $XDG_DATA_HOME environment variable. It's very OK to store data outside $HOME (like, on a NAS). Also, $HOME may not be stored in /home.

The default Unvanquished home directory on Linux is: ${XDG_DATA_HOME:-${HOME}/.local/share}/unvanquished, this can be anywhere.

If we really want some warning and things like that, I would prefer to require the user to pass a special argument like --yes-i-know-what-i-am-doing and abort (with a warning) otherwise if running with pid 0, or something like that.

If updater is running as root (pid 0), we may want to install to /opt/Unvanquished. When running the game as user the home dir would still be ${XDG_DATA_HOME:-${HOME}/.local/share}/unvanquished and then, writable. We would also have to install the .desktop file widely.

I think you misunderstood something. I never suggested the user should be forced to use a path inside $HOME. The condition I proposed for telling the user they must use a different path is that the updater is running as root AND they picked a path inside $HOME.

Hum, how would you know the user's $HOME if you're root? $HOME will be /root for root, which is not available to the user.

Right, there is some still-broken distros out there giving non-root $HOME environment variable to root user when using sudo, but this is a mistake of the distro which can break more than Unvanquished (bash history file, vim temporary files, X11-over-ssh, user cache, even prevent the user to login later…). Are we talking about that issue from distributions doing sudo the wrong way?

I did not know changing the environment variables is a functionality of sudo. I thought sudo is not supposed to run the profile script or anything, and keep the same environment of the calling user. I find on Mac the $HOME of the calling user is kept, but on Debian it changes as you say.

So instead of that, I may have it refuse to install if the user left the default install directory of /base.