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

Stop using process.umask() #18

Open
isaacs opened this issue Apr 3, 2020 · 0 comments
Open

Stop using process.umask() #18

isaacs opened this issue Apr 3, 2020 · 0 comments
Labels
Priority 2 will get attention when we're freed up

Comments

@isaacs
Copy link
Contributor

isaacs commented Apr 3, 2020

Re npm/cli#1103

Refs: nodejs/node#32321

Summary: process.umask() (no args) will be deprecated and removed.

I couldn't quite divine what lib/config/defaults.js uses process.umask() for but in most cases you don't need to deal with the umask directly - the operating system will apply it automatically.

Example:

const mode = 0o777 & ~process.umask();
fs.mkdirSync(dir, mode);

Computing the file mode that way is superfluous, it can be replaced with just this:

fs.mkdirSync(dir, 0o777);

Currently we use the process.umask() value to determine the appropriate mode for executable files on the system.

const fixBin = file => chmod(file, execMode)
  .then(() => isWindowsHashbangFile(file))
  .then(isWHB => isWHB ? dos2Unix(file) : null)

Since this is only done after the file is created (and it is created without the knowledge that it will eventually need to be an executable script), we can't just rely on default file creation masking, since chmod isn't limited by that.

If we don't read process.umask, we risk making all executable files world-writable (or even just group-writable) which is a security risk.

As I can see it, the only way to avoid this would be to have pacote take note of executable file targets at unpack time, and create them with a 0o777 mode, regardless of what the archive entry says, and then also tell tar not to chmod them to 0o777 after creation.

Probably this will require a way to provide chmod:false to tar.Unpack anyway, so that pacote can just set the creation modes to 0o666/0o777 and ignore the specific mode found in the archive.

cc: @bnoordhuis

@hashtagchris hashtagchris added the Priority 2 will get attention when we're freed up label Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority 2 will get attention when we're freed up
Projects
None yet
Development

No branches or pull requests

2 participants