[RRFC] Clean up file ownership story

[ruyadorno]

We currently have an entire category of issue reports around unexpected changing of file ownership. We need a place to start the discussion on what we can do to improve the story around file ownership management: Maybe we need less of it these days?

Opening this RRFC so that we can have a place to share thoughts, link related issues to and hopefully eventually evolve towards an RFC if we come up to the decision that changes are necessary.

cc @nlf @fritzy

[ljharb]

Auditing and aligning this sounds great.

I would expect that npm has full ownership (and thus, full freedom to change whatever it likes) of any node_modules, and $HOME/.cache, and things inside npm root -g, and its own installation folder.

Things a user is explicitly asking npm to change - .npmrc, package.json, lockfiles - npm of course can change, but shouldn't change ownership (it should error, if that is required; warn if not)

I would also expect that any other file is something npm is not "allowed" to change in any way.

[nlf]

AFAIK the existing file ownership changes are so that things like sudo npm install in a directory that's owned by your user doesn't write a bunch of files owned by root, both to your project and potentially to your cache.

This doesn't feel like something we should be attempting to forcibly fix on a user's behalf. The stat calls to determine ownership, and the chown calls to modify the ownership when it differs from what we infer are not free. We are most certainly taking a performance hit with these, especially in environments like Docker where filesystem i/o can be significantly slower.

Attempting to change permissions of things like the directory global bin scripts are located in can be absolutely disastrous.

Is there prior art for the type of permissions changes we attempt? I suspect there's not, but personally I also pretty strongly believe we shouldn't be attempting to change ownership ourselves.

[ljharb]

Is it possible to make sudo npm install immediately exit with an error?

[nlf]

potentially, but that wouldn't stop things like a user elevating their permissions to root manually and installing there. we absolutely can't explode when npm is run as root unconditionally, as that's often the case in a containerized environment.

[nlf]

a suggestion: what if npm doctor can help diagnose this class of problem? i.e. if some other npm command errors with something related to filesystem permissions, our error output can suggest the user runs npm doctor. that would then do things like examine the permissions of files in node_modules and the cache directory and either link to docs or display in-line how the user can correct the permissions themselves

[nlf]

after actually running npm doctor myself for the first time in quite a while, it already does permissions checks

Perms check on cached files         ok
Perms check on local node_modules   ok
Perms check on global node_modules  ok
Perms check on local bin folder     ok
Perms check on global bin folder    ok

so all that's missing is making our error output suggest running doctor, and doctor telling you how to fix the problem

[fritzy]

We could have a --no-root-user default for non-global installs, and then ci users would have to run --root-user. In the same release, we'd drop all ownership checks, and just write files as the current user. This would, of course, be a major breaking change.

ENOROOT: running `npm` as root is discouraged. Use `--root-user`to override. [link to docs]

[nlf]

adding this as another data point npm/cli#4312

[wesgarland]

Rather than dropping perms to nobody in this circumstance (which breaks compilation or yields surprising ownership issues), can we drop perms to $SUDO_USER instead?

…

Sent from my iPhone On Mar 3, 2022, at 19:08, Nathan Fritz ***@***.***> wrote:  We could have a --no-root-user default for non-global installs, and then ci users would have to run --root-user. In the same release, we'd drop all ownership checks, and just write files as the current user. This would, of course, be a major breaking change. ENOROOT: running `npm` as root is discouraged. Use `--root-user`to override. [link to docs] — Reply to this email directly, view it on GitHub <#546 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABTIUCYLNTMJ6UH4VVY2R3U6FH7LANCNFSM5P3WILHA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[theredcat]

Rather than dropping perms to nobody in this circumstance (which breaks compilation or yields surprising ownership issues), can we drop perms to $SUDO_USER instead?

I think that's not a good idea, not everyone is using sudo. If I ssh root@myserver this var won't be set because I'm "really" root. This is not a good practice, but it's the default on many hosters

[theredcat]

Adding two more references :

• [BUG] npm i -g foo changes ownership of files under /usr/bin cli#4010
• [BUG] npm resets the SUID bit for a binary in /usr/local/bin cli#3705