This repository has been archived by the owner. It is now read-only.

Critical Linux filesystem permissions are being changed by latest version #19883

Closed
Crunkle opened this Issue Feb 22, 2018 · 121 comments

Comments

Projects
None yet
@Crunkle
Copy link

Crunkle commented Feb 22, 2018

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 wrong?

This issue has been happening ever since 5.7.0 was released a few hours ago. It seems to have completely broken my filesystem permissions and caused me to have to manually fix the permissions of critical files and folders. I believe that it is related to the commit 94227e1 which is traversing and running chown on the wrong, often critical, filesystem files and folders.

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.

I found that a selection of directories in / were owned by a non-root user after running sudo npm and many binaries in /usr/bin stopped working as their permissions were changed. People experiencing this bug will likely have to fully reinstall their system due to this update.

npm update -g as root:

No output, all packages up to date. Likely still causes a chown to be run silently to root:root.
drwxr-xr-x 10 root root 129 Feb 22 03:39 /usr

Then doing a su jared (a non-root user):

sudo npm update -g as jared:

Sometimes EACCES or EPERM output, almost always corrupts the filesystem.
drwxr-xr-x 10 jared jared 129 Feb 22 03:39 /usr

The /usr directory has been claimed by npm and ownership was set to jared:jared as shown above. This same thing happens with other directories seemingly at random whilst being traversed.

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
...

It is very dangerous to run the latest version under sudo and I have a feeling it isn't just me getting these results.

How can the CLI team reproduce the problem?

I am personally using Arch Linux with the latest npm package, installed as the root user via:

pacman -Sy npm nodejs
npm install -g npm
npm install -g semver

Ensure that your npm is on version 5.7.0 then, as a non-root user, with sudo prefix:

sudo npm --help

You will find that it fails, sometimes with no warnings and sometimes with an EACCES as it is unable to chown the files in /boot or read-only directories. No log files are generated on my system as it throws an output in console.

This was not occurring on my system before the most recent update and using 5.6.0 resolves the issue entirely.

Supporting Information:

  • npm -v prints: 5.7.0
  • node -v prints: 9.5.0
  • npm config get registry prints: https://registry.npmjs.org/
  • Windows, OS X/macOS, or Linux?: Arch Linux (latest base)
  • Network issues:
    • Geographic location where npm was run: UK
    • I use a proxy to connect to the npm registry.
    • I use a proxy to connect to the web.
    • I use a proxy when downloading Git repos.
    • I access the npm registry via a VPN
    • I don't use a proxy, but have limited or unreliable internet access.
  • Container:
    • I develop using Vagrant on Windows.
    • I develop using Vagrant on OS X or Linux.
    • I develop / deploy using Docker.
    • I deploy to a PaaS (Triton, Heroku).
@welwood08

This comment has been minimized.

Copy link
Contributor

welwood08 commented Feb 22, 2018

I experienced similar chaos after updating to 5.7.0 in a FreeBSD jail, I noticed recursive ownership of at least /usr/local/etc had been given to the current user:group during the update (npm update -g). Thankfully I could rollback to a snapshot of my filesystem from before the update and 5.6.0 works fine again.

As a precaution against potentially catastrophic effects, I hope 5.7.0 will be taken down while a fix is implemented.

@juggy

This comment has been minimized.

Copy link

juggy commented Feb 22, 2018

This destroyed 3 production server after a single deploy!

@JSteunou

This comment has been minimized.

Copy link

JSteunou commented Feb 22, 2018

Same in a docker as root, some command like npm config set registry complained about EACCESS error on /root dir oO error code return is 7

All worked fine with 5.6 :(

@redboltz

This comment has been minimized.

Copy link

redboltz commented Feb 22, 2018

I experienced similar issue.
I'm using AWS ec2 (Amazon Linux AMI 2017.09.1 (HVM), SSD Volume Type - ami-f2d3638a).
It's clean environment. (Nothing installed.)

After npm v5.7.0 installed, /usr/bin's execution files ownership is updated to ec2-user:ec2-user from root:root. After the update, I cannot do sudo, so I need to re-create EC2 instance.

Here is the commands that reproduces the situation:

on ~ec2-user directory,

ls -l /usr/bin
# you can see correct ownership

sudo su
curl --silent --location https://rpm.nodesource.com/setup_6.x | bash -
exit

sudo yum update
sudo yum install git
sudo yum install nodejs
git clone https://github.com/isaacs/npm.git
cd npm
git checkout v5.7.0
make
sudo make install

ls -l /usr/bin
# you can see broken ownership
@pakastin

This comment has been minimized.

Copy link

pakastin commented Feb 22, 2018

Why are you using a pre-release version in production @juggy? Just asking...

@linaori

This comment has been minimized.

Copy link

linaori commented Feb 22, 2018

Tag doesn't look like pre-release until you actually click on the releases and check the tag. If it's a pre-release tag, I suggest to not format the tag in a way it looks like it could not be a pre-release tag.

@rpadovani

This comment has been minimized.

Copy link

rpadovani commented Feb 22, 2018

I suggest to do no publish pre-release version announcement on the official blog: http://blog.npmjs.org/post/171139955345/v570

@stefanotorresi

This comment has been minimized.

Copy link

stefanotorresi commented Feb 22, 2018

I suggest to do no publish pre-release version announcement on the official blog: http://blog.npmjs.org/post/171139955345/v570

...which doesn't mention it's a pre-release in any way, shape or form.

@AlexWayfer

This comment has been minimized.

Copy link

AlexWayfer commented Feb 22, 2018

Pre-releases is alpha, beta, rc, pre, etc. …

5.7.0 looks like minor stable release.

@kciredor

This comment has been minimized.

Copy link

kciredor commented Feb 22, 2018

lol

@pakastin

This comment has been minimized.

Copy link

pakastin commented Feb 22, 2018

Sorry people, wasn’t meant to be rude 🙄

@odykyi

This comment has been minimized.

Copy link

odykyi commented Feb 22, 2018

npm 5.7.0 on Mac not worked

@petetnt

This comment has been minimized.

Copy link

petetnt commented Feb 22, 2018

@towc welp, missed that 🆗

@linuxenko

This comment has been minimized.

Copy link

linuxenko commented Feb 22, 2018

haha )) can someone prove that this update does destroy production servers ?

@Crest

This comment has been minimized.

Copy link

Crest commented Feb 22, 2018

Oh yes

@vinicius73

This comment has been minimized.

Copy link

vinicius73 commented Feb 22, 2018

@linaori

This comment has been minimized.

Copy link

linaori commented Feb 22, 2018

Mind staying on topic, rather than enforcing your opinion on us for an unrelated subject? Thanks!

@Shredder121

This comment has been minimized.

Copy link

Shredder121 commented Feb 22, 2018

Interesting 🤔

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

@icedream

This comment has been minimized.

Copy link

icedream commented Feb 22, 2018

I think it would be preferable to only explicitly set permissions on folders that actually will be created by npm instead of just tinkering with the existing folders, especially when that code is accessing critical folders as the root user (through sudo). In the worst case, this renders systems unbootable and/or unusable.

On another note, it is indeed confusing to just use v5.7.0 as a pre-release version name despite the pre-release marker on GitHub. Or is the routine to mark upcoming releases as pre-releases and then just switching them over to stable release as-is?

@ansuz

This comment has been minimized.

Copy link

ansuz commented Feb 22, 2018

🍿

@tobozo

This comment has been minimized.

Copy link

tobozo commented Feb 22, 2018

consider reading this

@christianjgreen

This comment has been minimized.

Copy link

christianjgreen commented Feb 22, 2018

Playing a dangerous game by doing a minor stable release that's actually a pre.

@Tsutsukakushi

This comment has been minimized.

Copy link

Tsutsukakushi commented Feb 22, 2018

Things like this are like a 🍆 in the face of npm and node. And I love it.

@trodrigues

This comment has been minimized.

Copy link
Contributor

trodrigues commented Feb 22, 2018

For the people asking "why is 5.7.0 a prerelease released as stable", it's not:

{ latest: '5.6.0',
     next: '5.7.0'

It's tagged as next, so if you got it you either explicitly installed that version knowing it's marked as next, or you installed @next.

Release channels exist for a reason. If you're running "next" on production environments you're asking for trouble.

@elliotd123

This comment has been minimized.

Copy link

elliotd123 commented Feb 22, 2018

If what he's saying is correct, the pre-release appears to be pushed to the Arch main repositories, so if you're on Arch, beware! I've uninstalled npm and nodejs from my Arch box for the moment just to prevent any extra fun until this bug is fixed.

EDIT: it appears I misread his steps to recreate. Updating npm from npm is what installed the 5.7.0 version. Still bad, but not related to Arch's repositories.

@nickmccurdy

This comment has been minimized.

Copy link

nickmccurdy commented Feb 22, 2018

image

@JonasT

This comment has been minimized.

Copy link

JonasT commented Feb 22, 2018

@trodrigues might be still worth writing about this more explicitly on the blog post. Nobody coming to the site would know from reading it that it's a pre-release at the time.

Edit: for clarity, I'm referring to this: http://blog.npmjs.org/post/171139955345/v570

@mastertinner

This comment has been minimized.

Copy link

mastertinner commented Feb 22, 2018

I ran npm update -g this morning and my local npm 5.6.0 was upgraded to 5.7.0 so in some way, it must have gone beyond a pre-release.

@john-cullen

This comment has been minimized.

Copy link

john-cullen commented Feb 22, 2018

Generally in projects that follow semver I expect pre-release packages to have some string suffixed to the version number such as 5.7.0-next.

This is only listed as a MAY in the spec but it does allow you to immediately tell if a release is considered stable or not just from the version number.

@catwell

This comment has been minimized.

Copy link

catwell commented Feb 22, 2018

If what he's saying is correct, the pre-release appears to be pushed to the Arch main repositories, so if you're on Arch, beware!

I don't know where you see that, but it's not the case.

@julianlam

This comment has been minimized.

Copy link

julianlam commented Feb 22, 2018

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

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

Just so you know.

1 similar comment
@julianlam

This comment has been minimized.

Copy link

julianlam commented Feb 22, 2018

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

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

Just so you know.

@Loji

This comment has been minimized.

Copy link

Loji commented Feb 22, 2018

@k0nserv Still you can really mess up your dev machine. Things like this should never, ever happen. And they're happening a little too often to Node 😒

@ansuz

This comment has been minimized.

Copy link

ansuz commented Feb 22, 2018

I'm not heavily invested in this issue, but if you want this thread to feature more rational discussion, tweeting about it is probably a bad idea.

@rally25rs

This comment has been minimized.

Copy link

rally25rs commented Feb 22, 2018

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.

@ansuz

This comment has been minimized.

Copy link

ansuz commented Feb 22, 2018

I'm not heavily invested in this issue, but if you want this thread to feature more rational discussion, tweeting about it is probably a bad idea.

@LPCVOID

This comment has been minimized.

Copy link

LPCVOID commented Feb 22, 2018

Maybe this helps to dismantle the js bloat ecosystem which is slowly making the internet unusable.

@klaussfreire

This comment has been minimized.

Copy link

klaussfreire commented Feb 22, 2018

Its always nasty to have such problems. Besides all the talk/memes/etc., has anyone an idea how to test the fix and makes sure, that this kind of issue doesn't come up again in the future?

Yes, run it in a CI environment set up with selinux or apparmor configured to deny npm any action outside of its own work area. If it runs and doesn't break, chances are it won't corrupt the system. Maybe itself, but at least not the system.

1 similar comment
@klaussfreire

This comment has been minimized.

Copy link

klaussfreire commented Feb 22, 2018

Its always nasty to have such problems. Besides all the talk/memes/etc., has anyone an idea how to test the fix and makes sure, that this kind of issue doesn't come up again in the future?

Yes, run it in a CI environment set up with selinux or apparmor configured to deny npm any action outside of its own work area. If it runs and doesn't break, chances are it won't corrupt the system. Maybe itself, but at least not the system.

@k0nserv

This comment has been minimized.

Copy link

k0nserv commented Feb 22, 2018

@k0nserv Still you can really mess up your dev machine. Things like this should never, ever happen. And they're happening a little to often to Node. :(

You should be running the same versions of NPM/Node.js in dev as production so this is a non problem. The current LTS is 8.9.4 it comes with NPM 5.6.0. Just stay on a recent LTS release and use the NPM versions bundled with it.

@DaxDupont

This comment has been minimized.

Copy link

DaxDupont commented Feb 22, 2018

firerises

  1. Be a dev on one of the most critical pieces of on infrastructure of a widely used language.
  2. Release new prerelease.
  3. Do not use semantic versioning so it looks like a stable.
  4. Advertise new build in emails and blog posts but don't mention it's a prerelease.
  5. The update mechanism updates to next instead of latest for whatever reason
  6. It bricks everyone's systems.
  7. Users are reasonably upset.
  8. Insult users for being upset.

It might be time for an expansion of the NPM team and a review of the current developers on it.

@cjdelisle

This comment has been minimized.

Copy link

cjdelisle commented Feb 22, 2018

Thank you to everyone who is ranting about us 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 run npm unpublish npm@5.7.0 ?

@xPaw

This comment has been minimized.

Copy link

xPaw commented Feb 22, 2018

How about we give the two person team more than 24 hours to run npm unpublish npm@5.7.0

I'm not sure if you're joking, but that command only allows unpublishing versions published within 24 hours, and not older.

@eli-schwartz

This comment has been minimized.

Copy link

eli-schwartz commented Feb 22, 2018

That's okay, it was tagged 19 hours ago...

@npm npm locked and limited conversation to collaborators Feb 22, 2018

zkat added a commit that referenced this issue Feb 22, 2018

@zkat zkat closed this in #19889 Feb 22, 2018

dosire referenced this issue Feb 22, 2018

*: Switch from mkdirp to correctMkdir to preserve perms and owners
Switch to correctMkdir as much as possible to ensure that permissions on
directories are correct.

@npm npm deleted a comment from piedoom Feb 22, 2018

dos1 referenced this issue Feb 22, 2018

@zkat

This comment has been minimized.

Copy link
Member

zkat commented Feb 22, 2018

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

@mikesherov

This comment has been minimized.

Copy link
Contributor

mikesherov commented Feb 24, 2018

Looking back at my reaction this week, I realize that in the moment my responses were inappropriate and I have decided to delete the comment. Obviously, hiring decisions for any organization that I work for are based on wider evaluations of each individual’s background and qualifications. To anyone I may have offended, please accept my sincere apologies.

@npm npm locked as resolved and limited conversation to collaborators Feb 24, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.