Patched `childProcess.spawn` (and alikes) fail on executing `node -e 'code'`

[medikoo]

What version of pkg are you using?

5.6.0

What version of Node.js are you using?

14.9.0

What operating system are you using?

macOS

What CPU architecture are you using?

x86_64

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

node16-linux-x64

Describe the Bug

Running package with following code

childProcess.spawn("node", ["-e", "console.log('foo')"], { stdio: "inherit" });

Results with unexpected error:

./dist/index-macos 
node:internal/modules/cjs/loader:936
  throw err;
  ^

Error: Cannot find module '/Users/medikoo/Projects/_tests/repro-pkg-bug/-e'
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:933:15)
    at Function._resolveFilename (pkg/prelude/bootstrap.js:1912:46)
    at Function.Module._load (node:internal/modules/cjs/loader:778:27)
    at Function.runMain (pkg/prelude/bootstrap.js:1940:12)
    at node:internal/main/run_main_module:17:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

I believe it's due to patching args at https://github.com/vercel/pkg/blob/5.6.0/prelude/bootstrap.js#L2033 And it appears -e option is not handled as expected

Expected Behavior

Should print:

foo

To Reproduce

1. Package following code:

childProcess.spawn("node", ["-e", "console.log('foo')"], { stdio: "inherit" });

2. Run generated binary

[robertsLando]

PR?

[medikoo]

@robertsLando As I check, it doesn't seem trivial to fix.

The problem if I see correctly lies here: https://github.com/vercel/pkg/blob/8cd98af1ad7ab74df40fa9ffd5e6decdfb298969/prelude/bootstrap.js#L80-L82 It's not taken into account that node might be run with -e flag to evaluate the code.

In Node.js runtime above check is preceded with the handling of eval arguments: https://github.com/nodejs/node/blob/1a96d83a223ff9f05f7d942fb84440d323f7b596/lib/internal/bootstrap/node.js#L256-L266 Still, this logic seems to rely on functions not accessible outside of Node.js internals, so we can't just copy and paste it in.

What approach would you suggest?

[robertsLando]

What approach would you suggest?

The bug is for sure in those lines of code, unfortunally I have no quick solution to propose to you right now, you should make some tests your own. I suggest to create a dedicated unit test for this and run it to test changes you make in bootstrap code until it works

[jesec]

spawn routines of Node.js appear to interpret node as the executable itself.

In our case, it would make sense to remove that, and treat node the same way as any other executables in PATH.

This is not necessarily what you want, though. What are you trying to do here? Do you want a pkg-generated executable to function as Node.js runtime?

---

A simple test with notnode:

sudo ln -sf /usr/bin/node /usr/bin/notnode

then,

childProcess.spawn("notnode", ["-e", "console.log('foo')"], { stdio: "inherit" });

prints foo.

[medikoo]

This is not necessarily what you want, though. What are you trying to do here? Do you want a pkg-generated executable to function as Node.js runtime?

We bundle Serverless Framework and it has command to install plugins which under the hood runs npm (which is bundled together with a Framework), and in some plugin install scenarios npm install crashes when running postinstall scripts as https://github.com/medikoo/es5-ext/blob/v0.10.59/package.json#L115

[jesec]

This is not necessarily what you want, though. What are you trying to do here? Do you want a pkg-generated executable to function as Node.js runtime?

We bundle Serverless Framework and it has command to install plugins which under the hood runs npm (which is bundled together with a Framework), and in some plugin install scenarios npm install crashes when running postinstall scripts as https://github.com/medikoo/es5-ext/blob/v0.10.59/package.json#L115

🤔 Do you want the pkg-generated executable to evaluate that statement (that is, to function as the Node.js runtime)? Or it can be assumed that there is a node in PATH?

In the first case, you can add env: { PKG_EXECPATH: 'PKG_INVOKE_NODEJS' }, to your spawn.

In the second case, I will have to implement the change to make node non-special.

Note that the env variable hack is not officially supported. I am considering to remove the possibility of making a pkg-generated executable to behave as a Node.js runtime in a future major release.

---

Implementing the node from PATH implies removal of the PKG_EXECPATH, so that would be a major change.

[medikoo]

In the first case, you can add env: { PKG_EXECPATH: 'PKG_INVOKE_NODEJS' }, to your spawn.

I was thinking about that, but then it'll crash for users which don't have Node.js normally installed in their system.

It seems the only solid solution is to have node -e .. run with Node.js that's bundled with the executable.

[cruxcode]

I have the exact same problem. My software also wants the user to be able to install plugins, hence, I am bundling npm in the package, but, a few very widely used packages such as core-js-pure have postinstall script in their package.json with the value that looks something like this node -e "try{require('./postinstall')}catch(e){}". As a temporary work-around I have replace npm with yarn, because yarn, unlike npm. doesn't undo the installation when postinstall fails. There are other work around options available with yarn such as using dependenciesMeta in package.json or enableScripts in .yarnrc.yml.

Even with these work arounds, I am worried that this problem might break something in the future while developing some plugins etc. Upon reading this discussion, I think the work needed to resolve this issue lies in adding some logic to parse -e here as mentioned by @medikoo

https://github.com/vercel/pkg/blob/8cd98af1ad7ab74df40fa9ffd5e6decdfb298969/prelude/bootstrap.js#L80-L82

Maybe an additional change might be required here to load the module correctly.

https://github.com/vercel/pkg/blob/8cd98af1ad7ab74df40fa9ffd5e6decdfb298969/prelude/bootstrap.js#L1938-L1941

[cruxcode]

I have done some changes to my fork to show how the solution will look approximately. I have done a single commit, hence, it won't be difficult to compare changes.

To test the changes, change directory into examples/spawn-eval and run yarn run pkgExe && yarn run test. I have only applied one check and that is if /snapshot directory exists in the spawned process.

If these changes are in the right direction, I will put in some more time to make it perfect.

[github-actions[bot]]

This issue is stale because it has been open 90 days with no activity. Remove the stale label or comment or this will be closed in 5 days. To ignore this issue entirely you can add the no-stale label

[medikoo]

bump

[kubevious]

We are also experiencing troubles with this issue.

[jcs090218]

Any update on this? 😭

[github-actions[bot]]

This issue is stale because it has been open 90 days with no activity. Remove the stale label or comment or this will be closed in 5 days. To ignore this issue entirely you can add the no-stale label

[medikoo]

bump

[github-actions[bot]]

This issue is stale because it has been open 90 days with no activity. Remove the stale label or comment or this will be closed in 5 days. To ignore this issue entirely you can add the no-stale label

[jcs090218]

bump

[github-actions[bot]]

This issue is stale because it has been open 90 days with no activity. Remove the stale label or comment or this will be closed in 5 days. To ignore this issue entirely you can add the no-stale label

[jcs090218]

bump