Xsession.d script assumes that /sbin is in $PATH

Hi Johannes, thanks for your report.

I'm the one who proposed the use of /bin/bash, so I'm kind of disinclined to agree on switching back. ;) There were valid reasons for making that change.

Can't help thinking that you ask for trouble by excluding /sbin from $PATH, but possibly it's not just a mistake.

Other ways, besides those you mention, to deal with this bug might be:

* Change the upstart files so they call the upstart bin with full path.

* Add some condition to the .profile file in /etc/skel so it does not
  source ~/.bashrc when sourced by a DM.
  ( Got that idea from http://askubuntu.com/questions/591937 )

Subscribed Martin Pitt, who helped out with bug #678421.

When I was young I've been taught that the sbin dirs are not supposed to be in the user's $PATH, but only in the superuser's. (the "s" in /sbin stands for "superuser"). Usually in the sbin dirs are only tools you need for sysadmin tasks. So it is a good idea not to have it in the $PATH of an ordinary user.

I also doubt a little bit about changing /etc/skel/.profile as this would only affect new added users. All the already existing user accounts have their ~/.profile in their $HOME. That wouldn't be affected by changing /etc/skel/.profile

A solution I just tested is to unset BASH_VERSION in /usr/sbin/lightdm-session. Then standard ~/.profile wouldn't source .bashrc. I don't know if this has side effect though.

IMHO the simplest fix would just be to call it with full path, i. e. /sbin/upstart --user ... ?

On 2015-03-27 13:54, Martin Pitt wrote:
> IMHO the simplest fix would just be to call it with full path, i. e.
> /sbin/upstart --user ... ?

Ok.. Then, to address the OP's issue, we are talking about fixing upstart. Should we change the affected package to upstart?

Yes, I agree.

I have attached a new diff file for upstart together with a patch which shows the delta. This takes care of the potiential issue as far as upstart is concerned.

However...

@Johannes: /etc/X11/Xsession.d/99cadence-session-start does not belong to the upstart package, so this does not really fix your specific situation. Actually I don't see the cadence package anywhere in the Ubuntu archive. To fix it for yourself, you may want to edit that file, so it uses the full path when calling upstart. Preventing ~/.profile from sourcing ~/.bashrc is another way. (Any manual edit of /usr/sbin/lightdm-session will get lost at the next update of the lightdm package.)

Please post a new debdiff as we discussed on IRC, thanks! (setting to incomplete to remove from the list meanwhile)

Corrected patch attached.

The patch in #9 doesn't fix it completely for me, as also some of the scripts in /usr/share/upstart/sessions/* assume that /sbin is in $PATH. Some of them call initctl which resides in /sbin

Unfortunately this means that a lot of packages would need to be fixed.

A grep->loop->dpkg -S->awk->sort->uniq command gave on my machine the following list of affected packages.

avahi-daemon
cgmanager
dbus
gnome-keyring
gnupg-agent
indicator-application
kde-workspace-bin
logrotate
openssh-client
rsync
upstart-bin

Sorry for being that late about this. I only had time now to test and debug it.

In my opinion it's upstart's fault to put executables that are needed by non-root users into (/usr)?/sbin. That is against what I learned back in the 1990 when I started using UNIX / Linux.

Hi Johannes,

My own feeling is that Ubuntu doesn't distinguish as strictly between bin and sbin as you would like to see. This is the standard $PATH in the default /etc/environment:

PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games"

Possibly it can be justified by the fact that the root user is not enabled by default in a fresh Ubuntu install, but you are encouraged to use sudo when needed. After all, running stuff in sbin as a normal user don't typically do much harm, right?

Anyway, I think that some experienced developer should better comment on your objection.

The easiest ways to fix it for yourself are to either not dropping /sbin from $PATH in ~/.bashrc or prevent ~/.profile from sourcing ~/.bashrc.

Brw, would this symlink help?

$ ll /usr/share/upstart/sessions/initctl
lrwxrwxrwx 1 root root 13 apr 4 02:30 /usr/share/upstart/sessions/initctl -> /sbin/initctl*

OTOH I suppose that '.' must be included in $PATH for that to work, and that's not recommended.

This bug was fixed in the package upstart - 1.13.2-0ubuntu12

---------------
upstart (1.13.2-0ubuntu12) vivid; urgency=medium

  * debian/xsession.d/99upstart:
    Don't assume that /sbin is in $PATH when calling upstart
    (LP: #1435492).
 -- Gunnar Hjalmarsson <email address hidden> Fri, 03 Apr 2015 14:48:00 +0100

Hi Gunnar,

maybe I missed that ubuntu does interpret the meaning of /sbin differently than the classic UNIX systems. In that case I'd fix it in my .bashrc.

I really would be interested in a statement of an experienced ubuntu developer about such a decission and the reasons for it.

Anyway I'm a bit surprised, that the "fix" was released that quickly, given that it does not fix anything. Either my point that one cannot assume that /sbin is in $PATH is right. Then the fix is not sufficient. Or it is wrong, then my whole bug report is not justified.

In the former case the logical solution would be, that no executables that a non root user needs should reside in an sbin dir. That would affect the package upstart-bin, that provides /sbin/upstart as well as /sbin/initctl. upstart-bin also has some executables in /bin, so there seems to be some distinction between /bin and /sbin

Now we have a fix for a maybe not justified bug report that actually does not solve the problem. I actually would be happier with "won't fix" and a clear statement that also (/usr)+/sbin should be in the user's $PATH.

Hi again, Johannes.

Admittedly you make a few valid points. To get an authoritative statement about bin vs. sbin, I would suggest that you post a message to the ubuntu-devel-discuss mailing list, possibly referring to this bug report.

https://lists.ubuntu.com/mailman/listinfo/ubuntu-devel-discuss

I'm just a hobbyist contributor who make observations, and what I wrote in comment #11 is merely speculation. ;)

It's true that the uploaded patch does not fix the problem you brought up. Changing the status to "Confirmed" for now.