You know that feeling when you go chasing what you think is a relatively small bug, and it turns out to be a whole nest?
For v6.10.1, we addressed an issue with EISDIR errors when running as root. And, a fix in email@example.com addresses getting root-owned files in the cache when installing.
But I like sweeping fixes and tests where possible, and we have this nice big suite of integration tests.
On the isaacs/prevent-root-owned-cache-files branch, I’ve been working on this problem.
It’s a work-in-progress branch, and it’ll get rebased and cleaned up later. The commit history is a bit of a mess at the moment. But in a nutshell, this does the following:
- Update all the tests so that they use a known cache folder, instead of creating and removing their own cache folder. (Somewhat like the
common.pkgthat made parallel tests possible.)
- Fixed a few random cases (log files and metrics) that I knew wouldn’t be going through pacote.
- Added a
npm run sudotestscript so that you can run all the tests as root. (If this fails, you may need to so
sudo killall -9 nodeto stop any mock registries that your user account can’t kill directly.) (Took some work to make the tests all pass in this mode, because some rely on scripts being run, so they needed
- If the test is being run as uid 0, and there’s a
SUDO_GIDin the environment, then it scans that folder on teardown, and verifies that nothing in it is owned by uid 0.
Holy moly. A lot of stuff fails.
test/tap/access.js test/tap/adduser-always-auth.js test/tap/adduser-legacy-auth.js test/tap/adduser-oauth.js test/tap/adduser-saml.js test/tap/all-package-metadata-cache-stream-unit.js test/tap/all-package-metadata-entry-stream-unit.js test/tap/all-package-metadata-update-stream-unit.js test/tap/all-package-metadata-write-stream-unit.js test/tap/cache-shasum-fork.js test/tap/dist-tag.js test/tap/doctor.js test/tap/hook.js test/tap/link.js test/tap/logout-scoped.js test/tap/logout.js test/tap/optional-metadep-rollback-collision.js test/tap/org.js test/tap/owner.js test/tap/prepublish-only.js test/tap/publish-access-scoped.js test/tap/publish-access-unscoped.js test/tap/publish-invalid-semver-tag.js test/tap/publish-scoped.js test/tap/publish.js test/tap/search.all-package-search.js test/tap/search.js test/tap/sorted-package-json.js test/tap/team.js test/tap/unpack-foreign-tarball.js test/tap/update-examples.js test/tap/url-dependencies.js
It looks like anything using
npm-registry-fetch drops root-owned files in the cache when run as root.
I have to put this down until at least Monday or Tuesday. If anyone wants to help me dig in, it’d be great. My guess is that there’s a spot where we’re loading up npm-registry-fetch and need to be passing in a uid and gid.
I find it handy to run
npm run sudotest -- -b -s sudofails.txt || sudo killall -9 node to work through the tests and just re-run failures.
Sound like fun? This is probably the most common issue being reported on here these days, so let’s squash it once and for all.
(It’d also be good to get pacote and npm-registry-fetch themselves up to 100% test coverage.)
If you’re interested in helping, or just have clues or debugging to share, let me know. You can send a PR against https://github.com/npm/cli/tree/isaacs/prevent-root-owned-cache-files and I’ll pull those in as well. Everyone who pitches in gets an honorable mention in the 6.10.2 release notes, as well as the gratitude of a few million users