-
Notifications
You must be signed in to change notification settings - Fork 31
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: infer uid/gid instead of accepting as options
BREAKING CHANGE: the uid gid options are no longer respected or necessary. As of this change, cacache will always match the cache contents to the ownership of the cache directory (or its parent directory), regardless of what the caller passes in. Reasoning: The number one reason to use a uid or gid option was to keep root-owned files from causing problems in the cache. In npm's case, this meant that CLI's ./lib/command.js had to work out the appropriate uid and gid, then pass it to the libnpmcommand module, which had to in turn pass the uid and gid to npm-registry-fetch, which then passed it to make-fetch-happen, which passed it to cacache. (For package fetching, pacote would be in that mix as well.) Added to that, `cacache.rm()` will actually _write_ a file into the cache index, but has no way to accept an option so that its call to entry-index.js will write the index with the appropriate uid/gid. Little ownership bugs were all over the place, and tricky to trace through. (Why should make-fetch-happen even care about accepting or passing uids and gids? It's an http library.) This change allows us to keep the cache from having mixed ownership in any situation. Of course, this _does_ mean that if you have a root-owned but user-writable folder (for example, `/tmp`), then the cache will try to chown everything to root. The solution is for the user to create a folder, make it user-owned, and use that, rather than relying on cacache to create the root cache folder. If we decide to restore the uid/gid opts, and use ownership inferrence only when uid/gid are unset, then take care to also make rm take an option object, and pass it through to entry-index.js.
- Loading branch information
Showing
9 changed files
with
232 additions
and
90 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| 'use strict' | ||
|
|
||
| // This is only called by lib/util/fix-owner.js | ||
| // | ||
| // Get the uid/gid from the cache folder itself, not from | ||
| // settings being passed in. Too flaky otherwise, because the | ||
| // opts baton has to be passed properrly through half a dozen | ||
| // different modules. | ||
| // | ||
| // This module keeps a Map of cache=>{uid,gid}. If not in the map, | ||
| // then stat the folder, then the parent, ..., until it finds a folder | ||
| // that exists, and use that folder's uid and gid as the owner. | ||
| // | ||
| // If we don't have getuid/getgid, then this never gets called. | ||
|
|
||
| const BB = require('bluebird') | ||
| const fs = require('fs') | ||
| const lstat = BB.promisify(fs.lstat) | ||
| const lstatSync = fs.lstatSync | ||
| const { dirname } = require('path') | ||
| const inflight = require('promise-inflight') | ||
|
|
||
| const cacheToOwner = new Map() | ||
|
|
||
| const inferOwner = cache => { | ||
| if (cacheToOwner.has(cache)) { | ||
| // already inferred it | ||
| return BB.resolve(cacheToOwner.get(cache)) | ||
| } | ||
|
|
||
| const statThen = st => { | ||
| const { uid, gid } = st | ||
| cacheToOwner.set(cache, { uid, gid }) | ||
| return { uid, gid } | ||
| } | ||
| // check the parent if the cache itself fails | ||
| // likely it does not exist yet. | ||
| const parent = dirname(cache) | ||
| const parentTrap = parent === cache ? null : er => { | ||
| return inferOwner(parent).then((owner) => { | ||
| cacheToOwner.set(cache, owner) | ||
| return owner | ||
| }) | ||
| } | ||
| return lstat(cache).then(statThen, parentTrap) | ||
| } | ||
|
|
||
| const inferOwnerSync = cache => { | ||
| if (cacheToOwner.has(cache)) { | ||
| // already inferred it | ||
| return cacheToOwner.get(cache) | ||
| } | ||
|
|
||
| // the parent we'll check if it doesn't exist yet | ||
| const parent = dirname(cache) | ||
| // avoid obscuring call site by re-throwing | ||
| // "catch" the error by returning from a finally, | ||
| // only if we're not at the root, and the parent call works. | ||
| let threw = true | ||
| try { | ||
| const st = lstatSync(cache) | ||
| threw = false | ||
| const { uid, gid } = st | ||
| cacheToOwner.set(cache, { uid, gid }) | ||
| return { uid, gid } | ||
| } finally { | ||
| if (threw && parent !== cache) { | ||
| const owner = inferOwnerSync(parent) | ||
| cacheToOwner.set(cache, owner) | ||
| return owner // eslint-disable-line no-unsafe-finally | ||
| } | ||
| } | ||
| } | ||
|
|
||
| module.exports = cache => inflight( | ||
| 'inferOwner: detecting ownership of ' + cache, | ||
| () => inferOwner(cache) | ||
| ) | ||
|
|
||
| module.exports.sync = inferOwnerSync |
Oops, something went wrong.