NPM 5.7 recursively changing ownership of system directories when using sudo npm -g

powerlord

I just saw this linked on twitter.

To my knowledge this doesn't affect Windows. It has been confirmed to affect FreeBSD and multiple Linux distributions.

Critical Linux filesystem permissions are being changed by latest version · Issue #19883 · npm/npm

I'm opening this issue because: npm is crashing. npm is producing an incorrect install. npm is doing something I don't understand. Other (see below for feature requests): What's going w...

What's worse is that this is apparently a pre-release but is only marked as a pre-release if you look on their Github releases page. Everywhere else including its own version number, the npm blog, npm update, etc... refers to it as if it were a standard minor release.

~~Edit: Also, I put this in the wrong category. Whoops.~~ Boomzilla fixed it.

boomzilla

@powerlord said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

Edit: Also, I put this in the wrong category. Whoops.

Fixed. Also, wow wow wow!

By running sudo npm under a non-root user (root users do not have the same effect), filesystem permissions are being heavily modified. For example, if I run sudo npm --help or sudo npm update -g, both commands cause my filesystem to change ownership of directories such as /etc, /usr, /boot, and other directories needed for running the system. It appears that the ownership is recursively changed to the user currently running npm.

JazzyJosh

@powerlord

boomzilla

@jazzyjosh Where?

Also: https://github.com/npm/npm/issues/19883#issuecomment-367570304

juggy commented 10 hours ago
This destroyed 3 production server after a single deploy!

Because who doesn't deploy new stuff straight to production?

JazzyJosh

@boomzilla I meant I got Sorry.

boomzilla

https://github.com/npm/npm/issues/19883#issuecomment-367708728

marcan commented 18 minutes ago •
This isn't a bug, this is all working exactly as written and intended. There's a correctMkdir function that explicitly uses the sudo caller (not the effective or real user ID) to recursively chown any directory it is called to, and then this function is used all over the place, notably in places like the installation etc directory.

Apparently the npm developers feel they can do whatever they want with your system. Seriously, this isn't a subtle bug, this is code doing exactly what it claims to do. Which is stupid and clearly nobody tested this on a real system.

Now we can have a blakeydebate over the difference between a design bug and an implementation bug.

julianlam

Interesting...

// there's always a chance the permissions could have been frobbed, so fix

... oh dear... I think a big red flag was them moving away from mkdirp (a heavily relied-upon package) to correctMkdir, a hand-rolled lib in the npm repo. Ouch.

Just got the latest Node Weekly, whose first item is:

npm v5.7.0 Released
npm install can now automatically fix package-lock.json and npm-shrinkwrap.jsonfiles that have merge conflicts, there’s also a new npm ci command.

JazzyJosh

@boomzilla I think Marcan's edit is more important. This should be being addressed as

HOLY SHIT EVERYTHING IS ON FIRE ROLLBACK NOW

And 12 hours later it still isn't rolled back.

boomzilla

@jazzyjosh What did he say?

Does github run on node? I keep getting 0_1519315319112_44b5d829-38ac-4281-933c-1391afef33cf-image.png when I try to view the issue now.

JazzyJosh

@boomzilla

Edit: by the way, it took me literally 5 minutes of searching on the terrible GitHub interface to find this. I just searched for chown and followed the crumbs. You've had 12 hours. I don't do node.js and have never looked at the npm codebase before. Seriously, @mikesherov, there is no excuse for writing this in the first place, and even less of an excuse for not fixing it within the first 30 minutes after the report.

In reply to:

Thank you to everyone who is posting immature bullshit on this bug report. I now have a nice neat list of assholes I would never hire.
How about we give the two person team more than 24 hours to fix this bug?

ben_lubar

@boomzilla said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

Does github run on node?

Nope, GitHub runs on Discourse technology.

I'm getting the invisible pink unicorn too, but it works after a few refreshes.

julianlam

Despite his negative attitude, I do feel for him. Having the issue brigaded probably isn't helping him (or his team) debug and resolve... even if the solution is just a simple rollback.

Could be worse, this news could've broken on a Friday evening.

boomzilla

@ben_lubar said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

I'm getting the invisible pink unicorn too, but it works after a few refreshes.

I've refreshed a bunch.

@julianlam said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

Despite his negative attitude, I do feel for him. Having the issue brigaded probably isn't helping him (or his team) debug and resolve... even if the solution is just a simple rollback.

But they totally earned all of it and more. Hopefully they'll be more careful about this sort of thing in the future.

blakeyrat

@julianlam said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

Despite his negative attitude, I do feel for him.

You shouldn't. He broke a unknown number (but at last 4) of systems with this shitty, shoddy code that was never tested. The annoyance created to the owners of those systems is not somehow less important than the annoyance created to the developer.

The only lesson here is: don't release untested broken shit. But, of course, it's all OPEN SOURCEY to never test anything! RELEASE EARLY RELEASE OFTEN RELEASE BROKEN WHO CARES ABOUT USERS BREAK THEIR SHIT ALL THE TIME BROKEN BROKEN BROKEN!

Ugh. Should have went to barber school.

JazzyJosh

@julianlam said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

Could be worse, this news could've broken on a ~~Friday evening~~Monday morning at the beginning of business.

TheCPUWizard

In reply to:

Thank you to everyone who is posting immature bullshit on this bug report. I now have a nice neat list of assholes I would never hire.
How about we give the two person team more than 24 hours to fix this bug?

My response would be how about the "two person team" take responsibility for a failure to tests and accept the consequences. Unless there is something VERY surprising that has not surfaced, I would say that these two should be barred from ever writing code professionally or contributing to any repro.

JazzyJosh

@maharrg maharrg locked and limited conversation to collaborators 13 minutes ago

STILL NOT UNPUBLISHED

JazzyJosh

rally25rs commented an hour ago

I almost hate to contribute to this, but a hopefully informative bit of information:

This issue is made worse by the version tagging

latest: 5.6.0
next: 5.7.0

because npm upgrade does not take that into account and will pull the newest version (5.7.0).

Because of this, you should not npm upgrade -g npm or else you will get these pre-release builds. npm itself will tell you to run npm install -g npm isntead, which does pull latest and ignore that next is newer:

~ 🐒 npm -v
5.5.1

╭─────────────────────────────────────╮
│ │
│ Update available 5.5.1 → 5.6.0 │
│ Run npm i -g npm to update │
│ │
╰─────────────────────────────────────╯

~ 🐒 npm i -g npm
+ npm@5.6.0
added 27 packages, removed 11 packages and updated 38 packages in 7.544s

~ 🐒 npm -v
5.6.0

~ 🐒 npm upgrade -g npm
+ npm@5.7.0

added 63 packages, removed 6 packages and updated 49 packages in 8.432s```
~ 🐒 npm -v
5.7.0

so you can protect yourself from inadvertently getting these pre-release builds into our production environments by sticking to npm i -g.

JBert

@boomzilla said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

But they totally earned all of it and more. Hopefully they'll be more careful about this sort of thing in the future.

HAHAHAHA

I'm betting on Jeff-like ~~tactical retreat~~ lock & delete, then act like it never happened and continue whatever you were doing.

Zmaster

This is hilarious... a filesystem nuked with javascript. I've seen it all.

Cabbage

indeed. This distinction wouldn't be reasonable even if it were documented, but it ain't. (Or perhaps I'm just not clever enough to spot what difference in wording between https://docs.npmjs.com/cli/update and https://docs.npmjs.com/cli/install indicates that this difference exists. Regardless, for me, the effect is to same.)

dangeRuss

@julianlam said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

... oh dear... I think a big red flag was them moving away from mkdirp (a heavily relied-upon package) to corectMkdir, a hand-rolled lib in the npm repo. Ouch.

But it's correctMkdir, how can it be wrong?

They should've used mysql_real_escape_string

dkf

@dangeruss said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

But it's correctMkdir, how can it be wrong?

“I'mma gonna change your filesystem to be owned by me! Yay me!”

All this stuff makes me ever more happier to work with the Tcl community. They're a pretty conservative bunch that utterly loathe breaking things for people… (and have the problem of taking far too long to release anything, mostly because it's probably getting tested to hell and back first.)

julianlam

@blakeyrat said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

@julianlam said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

Despite his negative attitude, I do feel for him.

You shouldn't. He broke a unknown number (but at last 4) of systems with this shitty, shoddy code that was never tested. The annoyance created to the owners of those systems is not somehow less important than the annoyance created to the developer.

The only lesson here is: don't release untested broken shit. But, of course, it's all OPEN SOURCEY to never test anything! RELEASE EARLY RELEASE OFTEN RELEASE BROKEN WHO CARES ABOUT USERS BREAK THEIR SHIT ALL THE TIME BROKEN BROKEN BROKEN!

Ugh. Should have went to barber school.

I mean on a personal level. On a professional level, of course, he deserves to be taken to task for something like this. I'm just saying I empathise with the fact that he's probably not having a very good day right now.

My empathy has limits though... if the devs don't roll this back and he instead spends the rest of the day arguing on Twitter, I take it back.

(People on Reddit are pointing out that he's not an npm dev, so perhaps some of our anger is misdirected)

blakeyrat

@julianlam said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

(People on Reddit are pointing out that he's not an npm dev, so perhaps some of our anger is misdirected)

People white-knighting for others they have no relationship with is even worse.

Also my anger can't be misdirected because I'm angry at everything all the time.

Dreikin

@boomzilla said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

if I run sudo npm --help [the] commands causes my filesystem to change ownership of directories such as /etc, /usr, /boot, and other directories needed for running the system.

0_1519329909248_27752405_10211663932095974_3446357660287380307_n.jpg

...is a read-only command doing changing anything

TimeBandit

@blakeyrat said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

Should have went to barber school.

It's not too late

Dreikin

@powerlord said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

I just saw this linked on twitter.

To my knowledge this doesn't affect Windows. It has been confirmed to affect FreeBSD and multiple Linux distributions.

Critical Linux filesystem permissions are being changed by latest version · Issue #19883 · npm/npm

I'm opening this issue because: npm is crashing. npm is producing an incorrect install. npm is doing something I don't understand. Other (see below for feature requests): What's going w...

What's worse is that this is apparently a pre-release but is only marked as a pre-release if you look on their Github releases page. Everywhere else including its own version number, the npm blog, npm update, etc... refers to it as if it were a standard minor release.

~~Edit: Also, I put this in the wrong category. Whoops.~~ Boomzilla fixed it.

Oh my dog. It's even worse.

If you do not give it sudo permissions and just run npm alone, you can see it is attempting to traverse my /boot ownership and crashes when it fails (if given sudo, it will say chown instead of scandir and output an EACCES instead):
Error: EPERM: operation not permitted, scandir '/boot/initramfs-linux-fallback.img'
TypeError: Cannot read property 'get' of undefined
...

Error: EACCES: operation not permitted, chown '/boot/initramfs-linux-fallback.img'
TypeError: Cannot read property 'get' of undefined
...

What possible business does NPM have in /boot/?

TimeBandit

@dreikin said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

What possible business does NPM have in /boot/?

It can have this one

0_1519330894430_10630fae-d359-488e-a5d5-648cafb497f5-image.png

Adynathos

https://github.com/npm/npm/issues/19883 said:

FYI: npm@5.7.1 got released a few hours ago and resolves this issue. 5.7.0 will promptly fade into oblivion :) Cheers.

This comment perfectly summarizes the spirit of the discussion in that issue:

Too much peanut gallery here
saying that while I contribute to this useless posting

DCoder

bb36e

@blakeyrat said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

People white-knighting for others they have no relationship with is even worse.

Yes, it's terrible to display empathy for strangers

pie_flavor

@blakeyrat said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

@julianlam said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

(People on Reddit are pointing out that he's not an npm dev, so perhaps some of our anger is misdirected)

People white-knighting for others they have no relationship with is even worse.

Yes, nobody should ever stick up for anyone.

Lorne Kates

Sure, the package manager may randomly fuck up entire servers and burn everything to the ground, but if we didn't use it, I'd have to, like, write my own LeftPad function.

JBert

@pie_flavor said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

@blakeyrat said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

@julianlam said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

(People on Reddit are pointing out that he's not an npm dev, so perhaps some of our anger is misdirected)

People white-knighting for others they have no relationship with is even worse.

Yes, nobody should ever stick up for anyone.

Your strawman seems to be on fire.

Sticking up for someone is not always white-knighting.

pie_flavor

@jbert Neither is what @blakeyrat is describing.

bb36e

@jbert said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

Sticking up for someone is not always white-knighting.

>assuming you know what the shoulder aliens are telling blakey

Carnage

Soo, it's about time to put system level restrictions on npm in place? Such as locking it inside it's own little chroot, actively forbidding it admin rights throug any means at all (running it as root, running it with sudo and every other way there is).
Because it's provably a clusterfuck, in every concievable way. The actual package manager is maintained by a bunch of drunk monkeys hammering away at keyboards, and the packages it manages are worse ....

MrL

@carnage said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

Because it's provably a clusterfuck, in every concievable way. The actual package manager is maintained by a bunch of drunk monkeys hammering away at keyboards, and the packages it manages are worse ....

And all this in javascript land. I'm shocked.

Gąska

@lorne-kates said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

Sure, the package manager may randomly fuck up entire servers and burn everything to the ground, but if we didn't use it, I'd have to, like, write my own LeftPad function.

Funny thing - as mentioned above, the issue in this thread comes from npm devs switching from a well established library to their own implementation.

Jaloopa

@jazzyjosh said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

you should not npm upgrade -g npm

@jazzyjosh said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

run npm install -g npm isntead

@jazzyjosh said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

so you can protect yourself from inadvertently getting these pre-release builds into our production environments by sticking to npm i -g.

First off, why the fuck are there multiple similar commands to upgrade your version? Are you supposed to just know that upgrade will upgrade all the way to "next" whereas install` will "only" upgrade to "latest", which is clearly not as late as "next"? Oh, right. command line so that sort of thing is par for the course.

Second off, is it npm install -g npm or npm i -g? Is there a difference? Would it make some sort of sense to be consistent in informing people how to not upgrade to a broken version?

boomzilla

@jaloopa said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

First off, why the fuck are there multiple similar commands to upgrade your version? Are you supposed to just know that upgrade will upgrade all the way to "next" whereas install` will "only" upgrade to "latest", which is clearly not as late as "next"? Oh, right. command line so that sort of thing is par for the course.

Well, a GUI would have just done it and then nagged you until you restarted.

Cabbage

@jaloopa i is just an alias of install, as noted at https://docs.npmjs.com/cli/install. If you don't already know that, though, it's hard to find out, because i isn't listed in the CLI Commands section in the sidebar - only install is. Nor does https://docs.npmjs.com/cli/i redirect to https://docs.npmjs.com/cli/install; instead, it's just a 404 page.

(Indeed, after seeing npm upgrade mentioned in this thread, it took me several minutes of confusion to figure out WTF upgrade was and how it differed from update, which is the command I'm familiar with. They are, it turns out, aliases, as described at https://docs.npmjs.com/cli/update)

Aliases don't have to be documented shittily like this. Lodash (which I love) has a bunch of method aliases, and they're all listed in the sidebar at https://lodash.com/docs. The sidebar itself even makes clear that they're aliases before you click, by for instance listing _.first -> head (for the _.first method which is an alias of _.head).

Hmm. Maybe I'll pop open a pull request to fix this in the npm docs. Oh, wait; somebody already did: https://github.com/npm/docs/pull/679. It hasn't been merged in the two years since it was opened, though, or even acknowledged by a member of the npm team, because FYTW.

Jaloopa

@cabbage said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

Hmm. Maybe I'll pop open a pull request to fix this in the npm docs. Oh, wait; somebody already did: https://github.com/npm/docs/pull/679. It hasn't been merged in the two years since it was opened, though, or even acknowledged by a member of the npm team, because FYTW.

It's open source you can fix it yourself stop complaining etc.

TheCPUWizard

@jaloopa said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

@cabbage said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

Hmm. Maybe I'll pop open a pull request to fix this in the npm docs. Oh, wait; somebody already did: https://github.com/npm/docs/pull/679. It hasn't been merged in the two years since it was opened, though, or even acknowledged by a member of the npm team, because FYTW.

It's open source you can fix it yourself stop complaining etc.

Open Source does NOT mean that "just anybody" can make changes to the repository.....

Jaloopa

@thecpuwizard can you please tell that to the zealots who always insist that nobody has any right to complain about any open source product ever because they can fix it themselves? KTHXBAI

Lorne Kates

@boomzilla said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

Well, a GUI would have just done it and then nagged you until you restarted.

Well actually, a REAL GUI would have just rebooted the server on it's own without asking you.

Filed under: Windows 10, Idiot

TheCPUWizard

@jaloopa said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

@thecpuwizard can you please tell that to the zealots who always insist that nobody has any right to complain about any open source product ever because they can fix it themselves? KTHXBAI

There is a huge difference between "make a local change so that the problem does not impact you" [which is what Open Source allows] and the ability to "fix" the root cause so that other users of the original repository are not impacted.

izzion

@jaloopa
Even if you can't fix it globally by making changes to the repository, you can fix it for your self by coding the fix and then maintaining that monkey patch in perpetuity. Duuuuhhh.

Filed under: Why no, I've ~~totally~~ never had to do that for Asterisk, why do you ask?

Jaloopa

@thecpuwizard said in NPM 5.7 recursively changing ownership of system directories when using sudo npm -g:

make a local change so that the problem does not impact you apart from the fact that you have to maintain a custom build for ever and constantly merge any upstream changes