EISDIR error when invoked from cacache under sudo to fix ownership on MacOS

[godmar]

The npm.community site is dealing with a frequent failure mode whereby sudo npm install -g ... results in an EISDIR error on such paths as /Users/xxx/.npm/_cacache/index-v5/77/b2. They call it the EISDIR conundrum.

Thanks to one of the users on that site we were able to pinpoint this error and tracked it to where cacache invokes the chownr function to fix up the ownership of the directory it just created. I believe that npm 6.9.0 uses chownr 1.1.1, which would indicate that this code somehow ends up returning EISDIR via a callback to the caller (this is then turned via BB to the rejected promise visible in the error log.

Taken literally, the error would mean that chownr invokes the open(2) system call on the directory it is given on some execution path. The only path I could see that (on MacOS, where O_SYMLINK is defined) would be via lchown which has this implementation in node v8.x:

  fs.lchown = function(path, uid, gid, callback) {
    callback = maybeCallback(callback);
    fs.open(path, constants.O_WRONLY | constants.O_SYMLINK, function(err, fd) {
      if (err) {
        callback(err);
        return;
      }
      // Prefer to return the chown error, if one occurs,
      // but still try to close, and report closing errors if they occur.
      fs.fchown(fd, uid, gid, function(err) {
        fs.close(fd, function(err2) {
          callback(err || err2);
        });
      });
    });
  };  

I do not understand why this function calls fs.open() unconditionally. If the file in question is not a symlink, wouldn't this guarantee an EISDIR? According to the OpenGroup, lchown should behave like chown unless the object in question is a symbolic link. The code in question appears to work if and only if the object in question is a symbolic link.

Side note: chownr prior to v1.1.1 used chown, which does not perform this open call. I believe that the use of lchown was introduced by commit a631d84 which is included in v1.1.1 which ships in npm 6.9.0 as per package.json

I'll stop here. I realize from what I've written so far that if my interpretation is correct, this is a node.js bug (MacOS specific) and not a chownr bug, but I'll leave this here for future reference anyway.

[godmar]

PS: node v10 removed the in my eyes dubious implementation of lchown/lchmod on MacOS and replaced it with a libuv-based implementation. @snoack pointed out potential problems with this implementation as early as 2014, although the masses didn't get bitten by it in such large numbers until a631d84 landed which started using lchown on the sudo path for sudo npm install -g... I'm feeling more confident that my analysis is correct.

The decision to use lchown where available, including MacOS, as part of the response to #14 is what is adversely affecting the users of node v8.x on MacOS. A possible fix might be to use lchown only if node version >= 10.6, which is when libuv based lchown landed and the MacOS specific buggy implementation was removed.

[godmar]

See also issue 8203 in which a Mac user confirms my speculations.

[isaacs]

Landed in chownr 1.1.2. I'll update the dep in npm/cli for the next release.