Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Node 10, npm gives an EPERM on every operation #20112

Closed
paulvanbrenk opened this issue Apr 17, 2018 · 19 comments
Closed

Node 10, npm gives an EPERM on every operation #20112

paulvanbrenk opened this issue Apr 17, 2018 · 19 comments
Labels
confirmed-bug Issues with confirmed bugs. libuv Issues and PRs related to the libuv dependency or the uv binding. npm Issues and PRs related to the npm client dependency or the npm registry. windows Issues and PRs related to the Windows platform.
Milestone

Comments

@paulvanbrenk
Copy link

paulvanbrenk commented Apr 17, 2018

Version: v10.0.0-nightly201804175eb9f3c91c
Platform: Windows 10 64-bit and 32-bit
Subsystem: npm

Both node and npm are installed in default location, and on the path.

This works as expected in Node 9, which is why I filed an issue here.

Any npm command results in the following error on several machines:

fs.js:111
    throw err;
    ^

Error: EPERM: operation not permitted, open 'C:\Program Files (x86)\nodejs\node_modules\npm\bin\npm-cli.js'
    at Object.fs.openSync (fs.js:545:3)
    at Object.fs.readFileSync (fs.js:451:33)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:688:20)
    at Module.load (internal/modules/cjs/loader.js:589:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:528:12)
    at Function.Module._load (internal/modules/cjs/loader.js:520:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:719:10)
    at startup (internal/bootstrap/node.js:229:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:576:3)
fs.js:111
    throw err;
    ^

Error: EPERM: operation not permitted, open 'C:\Program Files (x86)\nodejs\node_modules\npm\bin\npm-cli.js'
    at Object.fs.openSync (fs.js:545:3)
    at Object.fs.readFileSync (fs.js:451:33)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:688:20)
    at Module.load (internal/modules/cjs/loader.js:589:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:528:12)
    at Function.Module._load (internal/modules/cjs/loader.js:520:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:719:10)
    at startup (internal/bootstrap/node.js:229:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:576:3)
@bnoordhuis
Copy link
Member

EPERM means "permissions issue." Could be a number of things, from an overzealous virus scanner to wrong file permissions.

@bnoordhuis bnoordhuis added windows Issues and PRs related to the Windows platform. npm Issues and PRs related to the npm client dependency or the npm registry. labels Apr 17, 2018
@jasnell jasnell added the question Issues that look for answers. label Apr 17, 2018
@paulvanbrenk
Copy link
Author

My guess is wrong file permissions are set by the installer, since Node 9 (both npm 5.6 and 5.8) work as expected. File location doesn't seem to be changed between versions, and this happens consistently on multiple machines, and keeps happening even after an hour.

@billti
Copy link
Contributor

billti commented Apr 17, 2018

I can repro the same issue easily. I tried to debug npm, but turns out this is before it even gets to running npm (as can been seen from the stack above, where it's just trying to open the npm-cli.js file to load it).

I have the latest v10.x branch built locally, and stepping through the code it appears to be due to the recent libuv, which is requesting write access to any opened file. As npm (along with the rest of Node.js) is installed under C:\Program Files, then write access is not granted (unless running with Admin rights).

You can see the change causing this here: ae2b5bc#diff-4a14951fd12c46802543f3a8aa54a620R437 . If I comment out that exact line ("access |= FILE_WRITE_ATTRIBUTES;") and rebuild, npm runs without issue.

This seems fundamentally busted on Windows if you can't run npm commands. (Odd/concerning if this has been in the v10.x branch for 15 days without being caught!)

@vsemozhetbyt
Copy link
Contributor

cc @nodejs/libuv @nodejs/platform-windows

@richardlau
Copy link
Member

Corresponding libuv PR: libuv/libuv#1777

cc @bzoz

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Apr 17, 2018

Can reproduce on Windows 7 x64.

  1. Create simple test.js file, then forbid write access via file property dialog (this should not be simple read-only property, but a security permission change).
  2. This file can be run by all current releases from v4 till v9.
  3. Last Nightly and v8-canary give EPERM: operation not permitted.

@vsemozhetbyt vsemozhetbyt added this to the 10.0.0 milestone Apr 17, 2018
@richardlau richardlau added confirmed-bug Issues with confirmed bugs. and removed question Issues that look for answers. labels Apr 17, 2018
@vsemozhetbyt
Copy link
Contributor

cc @jasnell as this can block v10 release.

@vsemozhetbyt vsemozhetbyt added the libuv Issues and PRs related to the libuv dependency or the uv binding. label Apr 18, 2018
cjihrig added a commit to cjihrig/libuv that referenced this issue Apr 18, 2018
This reverts commit aa1beaa.
This commit was causing EPERM errors in Node.js.

Fixes: nodejs/node#20112
@cjihrig
Copy link
Contributor

cjihrig commented Apr 18, 2018

PR to revert the libuv commit in libuv/libuv#1800. Unless a better solution is available quickly, I'd like to land the revert and cut a new libuv release.

@cjihrig
Copy link
Contributor

cjihrig commented Apr 18, 2018

@vsemozhetbyt can you PR a failing test case and mark it blocked?

@vsemozhetbyt
Copy link
Contributor

@cjihrig I am not sure. I can try, but if anybody is sure enough to make it quickly and confidently, please, do.

@richardlau
Copy link
Member

It's a bit late at night for me, but I think a rough test case would involve:

  1. Copying a .js file fixture to common.tmpDir
  2. Using the Windows icacls cli command to deny write access for the current user to the copied fixture
  3. Attempting to run the copied fixture with process.execPath

I suspect the complications will be getting the right parameters to icacls, and whether access needs to be restored so that the common.tmpDir can be cleaned up afterwards.

@jasnell
Copy link
Member

jasnell commented Apr 18, 2018

Ouch. @cjihrig ... we can float a patch if a libuv update cannot be landed this week. Really need to try to get the commits locked down by end of week.

@cjihrig
Copy link
Contributor

cjihrig commented Apr 18, 2018

Proposed release for tomorrow - libuv/libuv#1801. In addition to fixing this, I'd really like to get the fix for #19903 out.

@vsemozhetbyt
Copy link
Contributor

Trying also to read https://ss64.com/nt/icacls.html ...

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Apr 18, 2018

Strange.
As I cannot find an easy way to get file owner name to give it to icacls, I am trying to manipulate overall write permission via well-known SID.

When I deny writing permissions for all via GUI dialog, I can run with v4-v9 and not with v10.
When I deny writing permissions for all via:

execFileSync('icacls', [scriptName, '/deny', '*S-1-1-0:W']);

I cannot run with any version (Error: Cannot find module ...), while the permissions are identical.

Will investigate more if nobody will succeed sooner.

@billti
Copy link
Contributor

billti commented Apr 18, 2018

I just wrote up the below test, and this is performing as expected for me (i.e. fails with the current bits, works after building with my fix above).

I can turn this into a test-case and submit if you like. (I haven't contributed a test before, but I'm happy to give it a go). Let me know if you have any guidance on where/how this test should live, and any consideration that should be added.

const os = require('os');
const fs = require('fs');
const path = require('path');
const cp = require('child_process');

if (os.type() == 'Windows_NT') {
    // mymod.js should be a module in the same folder with default (inherited) permissions
    const mymodPath = path.join(__dirname, "mymod.js");

    // Removed any inherited ACEs, and any explicitly granted ACEs for the current user
    cp.execSync(`icacls.exe "${mymodPath}" /inheritance:r /remove "%USERNAME%"`);
    // Grant the current user read & execute only
    cp.execSync(`icacls.exe "${mymodPath}" /grant "%USERNAME%":RX`);

    // Attempt to load the module. Will fail if write access is required
    const mymod = require('./mymod');

    // Remove the expliclty granted rights, and reenable inheritance
    cp.execSync(`icacls.exe "${mymodPath}" /remove "%USERNAME%" /inheritance:e`);
}

@vsemozhetbyt
Copy link
Contributor

cc @nodejs/testing

@billti
Copy link
Contributor

billti commented Apr 18, 2018

Test up for review in #20116 (may want to merge this in with the fix to avoid any failing tests).

cjihrig added a commit to cjihrig/libuv that referenced this issue Apr 18, 2018
This reverts commit aa1beaa.
This commit was causing EPERM errors in Node.js.

Fixes: nodejs/node#20112
PR-URL: libuv#1800
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Bartosz Sosnowski <[email protected]>
@jasnell
Copy link
Member

jasnell commented Apr 18, 2018

Reopening until the libuv update lands here :)

@jasnell jasnell reopened this Apr 18, 2018
vsemozhetbyt pushed a commit that referenced this issue Apr 22, 2018
Adds a test-case to cover loading modules
the user does not have permission to write to.

Covers issue logged in #20112

PR-URL: #20138
Refs: #20112
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Bartosz Sosnowski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
jasnell pushed a commit that referenced this issue Apr 23, 2018
PR-URL: #20129
Fixes: #20112
Fixes: #19903
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
jasnell pushed a commit that referenced this issue Apr 23, 2018
Adds a test-case to cover loading modules
the user does not have permission to write to.

Covers issue logged in #20112

PR-URL: #20138
Refs: #20112
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Bartosz Sosnowski <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
bzoz added a commit to JaneaSystems/libuv that referenced this issue Apr 26, 2018
MylesBorins pushed a commit that referenced this issue Aug 7, 2018
PR-URL: #20129
Fixes: #20112
Fixes: #19903
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
MylesBorins pushed a commit to MylesBorins/node that referenced this issue Nov 5, 2018
PR-URL: nodejs#20129
Fixes: nodejs#20112
Fixes: nodejs#19903
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
MylesBorins pushed a commit that referenced this issue Nov 11, 2018
Backport-PR-URL: #24103
PR-URL: #20129
Fixes: #20112
Fixes: #19903
Reviewed-By: Myles Borins <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. libuv Issues and PRs related to the libuv dependency or the uv binding. npm Issues and PRs related to the npm client dependency or the npm registry. windows Issues and PRs related to the Windows platform.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants