Do not use NPM 5.7

#1

Post by jguimont » Thu, Feb 22, 2018, 5:18 AM UTC

Re: Do not use NPM 5.7

#2

Post by obw » Thu, Feb 22, 2018, 12:06 PM UTC

I find it interesting that nobody noticed this before public release. And apparently this version is a pre-release? But that isn't specified on the blog post?

Re: Do not use NPM 5.7

#3

Post by zaarn » Thu, Feb 22, 2018, 12:59 PM UTC

Excuse me, but what the fuck?

Looks like the line responsible checks if the npm binary is run as sudo and then uses the UID and GID of the invoking user when chowning the directory. [ https://github.com/npm/npm/blob/latest/lib/utils/correct-mkd... ]

I feel like screaming, who thought this was a good idea? If I invoke something as sudo, why does anyone think it should try to detect that and do anything about it? I want to run as the user sudo has set, not my own user, OBVIOUSLY.

Don't try to be smart about sudo, you will break stuff.

Re: Do not use NPM 5.7

#4

Post by lucideer » Thu, Feb 22, 2018, 1:21 PM UTC

This is really horrific.

The idea that correctMkdir() exists at all seems to me to be so wrong-headed.

This comment from the source says a lot:

    // annoying humans and their expectations!

Good UX is an important, oft-overlooked consideration, but there is definitely such a thing as taking it too far. If your humans are expecting this level of hand-holding, it's because you've trained them to expect it by pandering to them up until now. This is the kind of problem that should be handled with good, detailed, error message display when users don't get the result they expect, not "fixing" it with over-reaching magic.

I'm not sure I'd trust anything put out by the npm team in general from hereonin if they genuinely thought creating the correct-mkdir.js file in the first place was a reasonable idea. Is it? Genuinely open to a counter-argument.

Re: Do not use NPM 5.7

#5

Post by btown » Thu, Feb 22, 2018, 1:28 PM UTC

There appear to be no unit tests for their entire lib/utils folder. Which includes things like this (misguided) chown utility. https://github.com/npm/npm/tree/release-next/test - and note the lack of testing in the commit linked in the bug report.

I had an inkling that NPM was cancer, but not like this.

Yarn, by contrast, has everything you would expect of a Facebook-engineered library: https://github.com/yarnpkg/yarn/tree/master/__tests__/util

Will be closely evaluating a switch to Yarn for our live apps. This is simply sad.

Re: Do not use NPM 5.7

#6

Post by mkj » Thu, Feb 22, 2018, 1:30 PM UTC

Reminds me of a recent Yarn problem, overwriting which(1). https://github.com/yarnpkg/yarn/issues/4205

Re: Do not use NPM 5.7

#7

Post by Silhouette » Thu, Feb 22, 2018, 2:20 PM UTC

mkj wrote: post #6

Reminds me of a recent Yarn problem, overwriting which(1). https://github.com/yarnpkg/yarn/issues/4205

Both of these issues seem like a timely reminder that everyday Linux desperately needs a proper application management and security model.

Installing software where your options are

1. running as a regular user, and the install script can put whatever it wants within your user's directories

or

2. running as root, and the install script can do literally anything to anywhere on your system

is not fit for purpose, when the risks from both malice and incompetence are both reaching new heights almost daily.

These are systems we use for real work, but even smartphones and their toy app stores do better now. How do we still not have controls so applications can always be installed/uninstalled in a controlled way, can only access files and other system resources that are relevant to their own operation, and so on?

Re: Do not use NPM 5.7

#8

Post by grahamj » Thu, Feb 22, 2018, 2:26 PM UTC

btown wrote: post #5

There appear to be no unit tests for their entire lib/utils folder. Which includes things like this (misguided) chown utility. https://github.com/npm/npm/tree/release-next/test - and note the lack of testing in the commit linked in the bug report. I had an inkling that NPM was cancer, but not like this. Yarn, by contrast, has everything you would expect of a Facebook-engineered library: https://github.com/yarnpkg/yar…

"everything you would expect of a Facebook-engineered library"

So it collects your personal information, even when not using it, and uses it for profit?

Re: Do not use NPM 5.7

#9

Post by Piskvorrr » Thu, Feb 22, 2018, 2:28 PM UTC

obw wrote: post #2

I find it interesting that nobody noticed this before public release. And apparently this version is a pre-release? But that isn't specified on the blog post?

And even worse, 5.6.0 to 5.7.0 is, by semver, one minor point release to another minor point release - no breaking changes, no major bugs. 5.7.0-pre would raise some flags.

Re: Do not use NPM 5.7

#10

Post by Piskvorrr » Thu, Feb 22, 2018, 2:31 PM UTC

Silhouette wrote: post #7

mkj wrote: post #6

Reminds me of a recent Yarn problem, overwriting which(1). https://github.com/yarnpkg/yarn/issues/4205

Both of these issues seem like a timely reminder that everyday Linux desperately needs a proper application management and security model. Installing software where your options are 1. running as a regular user, and the install script can put whatever it wants within your user's directories or 2. running as root, and the install script can do literally anything to anywhere on your system is not fit for purpose, when…

FWIW, "new npm broke Æeeeverything? Meh. Destroy the docker container, force version <= 5.6.0, rebuild" has now saved me from a bigger disaster. This is the 1.5th option, IMNSHO: npm gets its root(-ish) access, host computer is somewhat protected.