yao-pkg / pkg

Package your Node.js project into an executable
https://www.npmjs.com/package/@yao-pkg/pkg
MIT License
312 stars 11 forks source link

Permission problems with /tmp/pkg directory when multiple users use packaged binaries with native node modules #51

Closed kayahr closed 4 months ago

kayahr commented 4 months ago

What version of pkg are you using?

5.11.5

What version of Node.js are you using?

20.12.0

What operating system are you using?

Debian 12

What CPU architecture are you using?

x86_64

What Node versions, OSs and CPU architectures are you building for?

node20-linux-x64

Describe the Bug

During the bootstrap process of a packaged binary native node modules are extracted into /tmp/pkg. The problem with this approach is that this directory is globally used and the directory is created with the current user as owner. So when a second user also runs a different binary packaged with pkg or a new version of the binary including an updated native node module then the program fails with this error message:

EACCESS: permission denied, mkdir ‘/tmp/pkg/….’

Expected Behavior

It should be possible to use binaries including native node modules packaged with pkg from multiple users on the same machine.

To Reproduce

Alternative:

kayahr commented 4 months ago

This can easily be fixed and I can provide a PR for it but I'm not sure what is the best solution:

  1. Change permissions on /tmp/pkg to 0777 when it is initially created.
  2. Use UID in temporary directory name. Like /tmp/pkg-1000/. This would be the easiest solution I guess.
  3. Don't use a tmp directory, use a cache directory instead. On Linux this would be $HOME/.cache/pkg. No user conflict because each user has its own cache directory. Not sure where other operating systems store files like this.
robertsLando commented 4 months ago

Feel free to open a PR, I would not use a cache dir as it doesn't ha ve a cross OS support. The best sollution IMO is to use the 1.

kayahr commented 4 months ago

The problem with the first solution is, that it requires an explicit call to chmod. It is not possible to simply pass the permissions to mkdir. The default permission is already 0777 but it is masked by the global umask so with the standard umask 0022 it is downgraded to the current 0755, which causes the problem.

But explicitly calling chmod can fail in specific situations. For example when some application which used a previous pkg version already created a /tmp/pkg directory with a different user but the permission problems are not important because the native module never changes anyway. When the new pkg version now uses chmod on /tmp/pkg then the call will fail because the current user has no write permission to it. So we might consider wrapping the call with try...catch to ignore any errors here but this is very ugly.

That's why I think adding the User ID to the directory name is the simpler solution. On Windows the user ID is always -1 and the temp directory is per-user anyway, so by only appending the ID when it is >=0 this only applies to unix-like operating systems. And it is backward compatible and automatically fixes already existing problems, because an application using the new pkg version does not use a potentially existing and broken /tmp/pkg directory anymore.

kayahr commented 4 months ago

Another reason against the first AND the second solution (or the general idea of using a global cache in /tmp) is security related:

Let's say there is a Node.js application packaged with pkg using node-canvas running under user A (Maybe even root) on a machine. And lets say there is an evil user B on the same machine (or some other security issue allows remote users to execute code under user B). When user B learns that this application loads executable code from a publicly writable /tmp directory then user B could place a modified node-canvas binary with the correct hash filename in this directory and when user A runs the application it happily executes the hacked binary.

This problem does not exist when using a cache directory in the users home directory. Instead of $HOME/.cache/pkg it is also possible to use $HOME/.pkg/cache for example which would also work fine on Windows without doing OS detection. Tools like NPM, SSH and Git already use dot-prefixed files and directories in the users home directory so this concept is not that foreign to windows.

EDIT: Just noticed that on my windows machine there is already a $HOME/.cache directory. Created by Puppeteer to cache browser binaries (in $HOME/.cache/puppeteer of course). Perhaps this can serve as a precedent and we can simply use $HOME/.cache/pkg everywhere without doing OS detection.

robertsLando commented 4 months ago

EDIT: Just noticed that on my windows machine there is already a $HOME/.cache directory. Created by Puppeteer to cache browser binaries (in $HOME/.cache/puppeteer of course). Perhaps this can serve as a precedent and we can simply use $HOME/.cache/pkg everywhere without doing OS detection.

Ok so go for that