init: support mandatory arguments, or prevent starting of tasks without any arguments

I can confirm this problem (completely updated Lucid). The behavior of the system when you delete all you files is acutally quite interesting, see attached screenshot.

Sorry, the only response here is "Don't Do That Then"

The script is not intended to be run manually, and as you've discovered, when run without setting MOUNTPOINT it will wipe your root filesystem.

For testing, you should have done:

  start mounted-tmp MOUNTPOINT=/tmp

@3

How do you distinguish scripts that can be run manually and which shouldn't? Is there *any* documentation, that you should use

start mounted-tmp MOUNTPOINT=/tmp

for testing?

The problem is, that you don't know this script is dangerous before you executed it.

And yes, it is a bug since the script does not check whether MOUNTPOINT is set or not. This is what I would define as buggy and unsafe behavior.

So, just to clarify that:

What you are basically saying is that the manpage stating

> start JOB...
> Requests that the named jobs be started. The status of the jobs will be output to standard output until
> they are succesfully running, or in the case of tasks, until they have completed.

should in fact be read as: "might either start the job or delete all your data, and we neither check, warn or document regarding this issue anywhere visibly"? Seriously?

In my opinion, it is very definitely a wrong behaviour of any init script to erase the whole filesystem depending on the setting of an environment variable. The script should use sensible defaults if the variable is not set externally, or at least should exit gracefully in this case. Doing so in a shell script is trivial.

On Wed, 2010-04-07 at 14:12 +0000, Thomas Krause wrote:

> The problem is, that you don't know this script is dangerous before you
> executed it.
>
Then you should not execute it.

Scott
--
Scott James Remnant
<email address hidden>

On Wed, 2010-04-07 at 14:21 +0000, Torsten Jerzembeck wrote:

> In my opinion, it is very definitely a wrong behaviour of any init
> script
>
This is not an init script.

Scott
--
Scott James Remnant
<email address hidden>

@7

Hopefully "apache2" is never converted to upstart, otherwise I will never ever again execute the upstart-equivalent of

/etc/init.d/apache2 restart

because I'm afraid it will destroy my installation and my data (or at least I can never know...)

@3 (again)

sudo initctl reload network-interface

gives me the message

initctl: Unknown parameter: INTERFACE

Obviously the "network-interface" job is able to detect unset parameters instead of destroying everything.

On Wed, 2010-04-07 at 15:39 +0000, Thomas Krause wrote:

> sudo initctl reload network-interface
>
> gives me the message
>
> initctl: Unknown parameter: INTERFACE
>
> Obviously the "network-interface" job is able to detect unset parameters
> instead of destroying everything.
>
Yeah, I was just thinking about this over a cup of tea. instance jobs
already have mandatory parameters, since they're required to determine
which instance it is.

It makes sense to extend this to tasks as well, which require
environment from their events. Even /etc/init/rc.conf technically
requires RUNLEVEL be set.

If we handle this at the Upstart-level, it makes more sense. So
something like:

  # start mounted-tmp
  start: Missing argument: MOUNTPOINT

Then you're more likely to do:

  # start mounted-tmp MOUNTPOINT=/tmp

Scott
--
Scott James Remnant
<email address hidden>

With all due respect, this is more serious than a "Wishlist" item. A script that can be executed which will destroy a server's filesystem is a serious problem. I cannot think of any usage case where it would be preferable for a script to destroy the entire filesystem. Additionally, the script name of "mounted-tmp" would imply to any reasonable person that this script only affects the tmp directory. Having it fail to catch an error resulting in the destruction of the entire filesystem is simply not acceptable. This is the kind of thing that makes people look at linux and say it's not enterprise-ready. Please reconsider patching this flaw.

I must say I find the response of the maintainer here totally unacceptable. This bug should be marked release-critical not wishlist. The bug-fix should be released to the ubuntu security distributions. What is the point of protecting your system from outside attacks when the system itself will attack you?

What is the point of screwing around changing priorities and importance here on launchpad, when you could easily fix the script in 30 seconds?

I don't know anything else about Mr. Remnant, if he is just having a bad day, or whether he is generally a bad maintainer.

Being a maintainer does not give you the right to be a BOFH - you have a duty to do a good job and respect valid bug reports. As this fault caused extreme destruction to the system of at least one user, you should consider it an extremely serious bug. I do not recall a more serious bug report. I don't care that much if X won't start once in a while, but I care a whole lot if my whole system gets erased when I run a program without arguments.

This is an init script that any sysadmin might run, expecting it to do something normal. It is not okay for it to trash the filesystem. Even the infamously dangerous "rm" and "rsync" commands do not trash your entire filesystem when run without arguments.

If Canonical had an email address to "complain about a maintainer", I would be complaining right now. (and will do so if I find out about such an address in future).

The maintainer here is evidently hasn't a clue about anything. I'm no veteran but I've worked with several varieties of UNIX OSes throughout the years and NONE of them have ever had a plain executable destroy a filesystem without corresponding arguments. The user has no idea of when an executable is to be used by an application or by the end user (because all executables are user-runnable, duh), and all systems allow the usage of such since many tasks REQUIRE that you get down and dirty into system internals.

This behavior is inexcusable and I cannot consider Ubuntu to be a serious server OS if it doesn't have decent guidelines for the behavior of its system maintenance executabes.

From: http://www.ubuntu.com/education

"Above all, Ubuntu is set apart from other operating systems by its unwavering focus on simplicity and ease of use. Ubuntu's motto is "Linux for Human Beings" and every development decision and application has that goal in mind."

EPIC FAIL! hahaha

If we can't get anything more than a dismissive attitude from Mr. Remnant, Mark Shuttleworth might care about this bug. Contact details are available on his blog at http://www.markshuttleworth.com/contact-details

Scott: in case you are wondering why abuse is suddenly being heaped on you, it's because the moral outrage brigade from Reddit and Hacker News have come to exercise their moral outrage on something that has never touched them, will never touch them, have no responsibility over and nor desire to help out in any way shape or form other than to attack you. Some people think a comment box and a submit button are a license to say whatever's on their mind whenever they want.

Reddit/Hacker News types: hey, you guys see a downvote button anywhere? No? Hrm, maybe you've gotten lost and need to find your way home and let the guy do his job.

Actually Bjorn, I know about this bug through Microsoft employees throwing it in my face. If you think Microsoft won't use this against Ubuntu (and/or Linux in general) you've got another thing coming. This is a bug that affects every Ubuntu user and system administrator out there. Being dismissive about a flaw which causes data loss is only going to further the anti-linux cause. I would be happy to let the guy do his job if he acknowledged that he was going to do it. Right now, I see the maintainer marking this as "Invalid" and "Wishlist". If someone takes responsibility for fixing it, and acknowledges the seriousness of the problem, i'm done here.

Could people please read all the comments before judging so quickly? I was thinking the same thing at first, but if you read message #10 he rethinks his position and marks it to be fixed. I would be more likely myself to mark it as critical rather than wishlist, but the maintainer has acknowledged the problem and it will get fixed, no need to jump all over him now.

Here is a patch to mounted-tmp.conf (part of the 'mountall' package) that will set a sane default if $MOUNTPOINT isn't defined.

from:
http://news.ycombinator.com/reply?id=1248952&whence=item%3fid%3d1248725

I have a patch

   if [ -z $MOUNTPOINT ]; then
    echo "MOUNTPOINT not defined. Not deleting your file system.";
    exit(1);
   fi

This patch will be included in Zippy Zebra due out in 2018

Whether or not a fix to upstart (Scott, #10) gets implemented, please fix mounted-tmp.conf as well (e.g. per Chris, #20). The current version is way too dangerous to have lying around. AndyOsi (#13) is correct that such landmines are not to be expected in a *nix system.

I've submitted a one-line change to make this fail safely in https://code.edge.launchpad.net/~mdz/ubuntu/lucid/mountall/bug-557177

Wow.

You guys need to chill out ;-)

I'm sorry that you got confused that moving the bug from "upstart (Ubuntu)" to just "upstart" involved marking it as _Invalid_ and opening a new task; I think that's one of Launchpad's more idiotic design flaws too.

I'm also sorry that I receive literally hundreds (sometimes over a thousand) of bug comments a day that I have to deal with, so am often a little bit terse in replying - I have to maintain just about every line of code between the Ubuntu kernel and the X server and basically the entire boot system, and that includes all the bugs. I just don't have the time to post detailed responses detailing my every thought process!

Rather than ask Mark to personally intervene, perhaps you could ask him to hire more people to help me out? :-)

Anyway, as should be obvious here if you take a Valium first, I think that this is something Upstart should deal with itself - it should make it possible to write tasks and services without needed hundreds of lines of identical sanity checks in the script for every single one. One of my goals is to make it so that the Upstart job format means you only ever need specify the "meat" of what you want done, not all of the associated crap that init scripts used to have.

This functionality is "Wishlist", but please don't confuse that with "Low priority". Wishlist to me means that it's a feature that Upstart doesn't support but should, rather than a broken existing feature. (I close wishlist bugs that I disagree with)

Also please credit me with some intelligence here, I know that this has been linked from Planet Debian so I should expect flaming idiocy, but I don't think I have the merit and reputation that I have within Ubuntu from sitting with a thumb up my arse rather than working to improve the distribution.

A workaround for mountall was committed *yesterday* before any of you ever heard about this: http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/lucid/mountall/lucid/revision/298 - this sets a default value for $MOUNTPOINT and thus removes the "surprising" side-effect for these scripts.

Now, there's rather a lot of toys on the floor here, if people could pick them up and put them back in their prams I'd appreciate that - it looks untidy!

(it got pointed out to me that this bug was missing the reference to the mountall package, which has the bug fix, added)

@Scott: thank you for the explanation in #24. Those few minutes to write this comment we're well spent. Did get a wrong impression because of the marking as invalid, too...

Even though you're paid to work on Ubuntu, I'd like to thank your for all the ways you've made Ubuntu a better distro;-)

with all due respect to scott, it wasn't just the "--> Invalid" that caused this tempest. it was the somewhat dismissive sounding 'Sorry, the only response here is "Don't Do That Then"', which helped reinforce the impression that this was a "won't fix" situation.

but that aside, this thread was at least the second of the day in which the marking of the "(ubuntu)" version of a bug invalid caused confusion. it threw me off in https://bugs.launchpad.net/ubuntu/+source/upstart/+bug/94065/comments/6 , even _with_ scott's explanation of what was going on. if it were possible to substitute something less "terminal" sounding than "Invalid" as the disposition in cases like this ("Reclassified"? "Moved"? "Wrong 'affecting'"?), it might help avoid future confusion. (i realize there's probably limited control over the nomenclature choices.)

Mr. Remnant, you have an incredibly condescending attitude and your comments have ensured that I will seek alternatives to upstart and Ubuntu in general for my deployments.

Insulting competing websites, insinuating that people are children for being upset about WIPED FILE SYSTEMS, mocking them etc. is in poor taste and though I do not doubt your coding skills, I doubt your PR skills.

@Matt: I lol'd

Count me in support for sjr here. It's a thankless task to maintain software--every fix like this has to be weighed in priority against all the other bugs people are complaining about, and it seems like the bugmail never stops--and I know the temptation to respond tersely. On the other side, I'd also succumbed to the temptation to "escalate to Slashdot" when a bug I care about doesn't seem to be getting enough attention. The escalation was in questionable taste in this case: like it or not, the root user in Unix does have the authority to destroy the filesystem i he makes an accident. Be careful when you're root!

I'm glad the immediate bug was fixed--and so quickly!--and I hope that he wishlist bugs eventually get their attention, too -- both upstart growing the ability to check for mandatory arguments, and also for launchpad to distinguish reclassified bugs without confusing unfamiliar readers by marking them "Invalid".

Just to clarify: Neither I nor Thomas escalated this bug to any list - especially because I never experienced bringing in the trolls helps in any way. Besides: There are other ubuntu issues that permanently annoy me again, while I am very certain that I will never again run in the bug described here ;)

While I have to admit that I was quite pissed by the initial reply, I am really happy with the result. Not just because of the fast fix, but because of sjr rethinking the situation and then doing the fix. Most maintainers and developers tend to stick by their initial solution, so kudos for that reaction. Thanks.

Re: Chris Ladd's comment #20

Is there any possibility that this script, as patched, still fails catastrophically if MOUNTPOINT is defined to a non-existent directory, or if /tmp doesn't exist?

(The Launchpad flaws that resulted in people thinking this bug report had just been marked "Invalid" instead of merely refiled are, in order of proximity, bug 558822, bug 80902, and bug 76416.)

On Thu, 2010-04-08 at 23:17 +0000, dedded wrote:

> Is there any possibility that this script, as patched, still fails
> catastrophically if MOUNTPOINT is defined to a non-existent directory,
> or if /tmp doesn't exist?
>
The "cd ${MOUNTPOINT}" will fail, and the script will exit; that's why
it's written to do that, rather than assume that the mountpoint exists.

Upstart runs all shell scripts with "set -e" by default

Scott
--
Have you ever, ever felt like this?
Had strange things happen? Are you going round the twist?

> Upstart runs all shell scripts with "set -e" by default.

So what if the script is *not* run by upstart, but by somebody trying to figure out why things work (or not)?
If you need to assume that a piece of shell code is executed with -e being set, add a set -e before that piece of code. Don't assume it will be called with sh -e...

This bug was fixed in the package mountall - 2.11

---------------
mountall (2.11) lucid; urgency=low

  [ Scott James Remnant ]
  * conf/mounted-*.conf: Add defaults for $MOUNTPOINT just in case somebody
    tries to run this by hand. LP: #557177.

  [ Dustin Kirkland ]
  * conf/mounted-varrun.conf: drop initial motd creation, as this is now
    handled entirely dynamically by /etc/update-motd.d/* scripts, with
    this part specifically handled by base-files's 00-header,
    LP: #516293
 -- Dustin Kirkland <email address hidden> Fri, 09 Apr 2010 13:50:29 -0500

On Fri, 2010-04-09 at 13:39 +0000, Bernd Zeimetz wrote:

> > Upstart runs all shell scripts with "set -e" by default.
>
> So what if the script is *not* run by upstart, but by somebody trying to figure out why things work (or not)?
> If you need to assume that a piece of shell code is executed with -e being set, add a set -e before that piece of code. Don't assume it will be called with sh -e...
>
The syntax of these files is such that they can only be run by Upstart,
unless you copy & paste the shell fragment out and run it directly --
but then you're missing a lot of the set-up that Upstart does for you.

So that's really not recommended ;-)

Scott
--
Have you ever, ever felt like this?
Had strange things happen? Are you going round the twist?

> > you don't know this script is dangerous before you
> > executed it.
>
> Then you should not execute it.

A single bug in let's say firefox may delete your $HOME.
Do you study every line of code before you launch it?

dear scott, as you failed to see the problem in having a script in upstart that recursively wipes all file systems when called according to the man page I would rather not have upstart on my systems. please make upstart an optional feature and let people stick with the robust sysv init where a script wiping your server definitely *is* a bug. upstart has no advantages for me but comes with lots of problems (for me at least bugs #62751, #416056, and #447747).

On Fri, 2010-04-23 at 17:02 +0000, Thomas Zehetbauer wrote:

> dear scott, as you failed to see the problem in having a script in
> upstart that recursively wipes all file systems when called according to
> the man page I would rather not have upstart on my systems.
>
I not only fixed the problem with the mountall script, but have kept
this bug open because I consider it a bigger problem that should be
addressed across the board.

If you don't want Upstart on your system, don't use Ubuntu, Fedora, RHEL
or OpenSuSE. Have fun.

Scott
--
Have you ever, ever felt like this?
Had strange things happen? Are you going round the twist?

Scott, Your doing a great job and appreciate the patch. Ubuntu is a great OS and I have switched all my PCs and families PCs over and created two servers with Ubuntu. It is in fact a great OS system.

James

> Upstart runs all shell scripts with "set -e" by default

IMHO, Upstart should also "set -u" by default.