Invalid package error when there is asar path in the command passed to child_process.exec

[jviotti]

I'm hitting this strange issue when using the ELECTRON_RUN_AS_NODE=1 feature to run a script inside an asar, which re-runs itself with child_process.exec() as part of an elevation routine provided by sudo-prompt.

I've created a small snippet that reproduces the issue here. I tried creating a Gist, but it doesn't allow me to upload asar archives.

Installing the npm dependencies and running npm start outputs the following:

$ npm start

> electron-elevate-asar@1.0.0 start /Users/jviotti/Projects/playground/electron-elevate-asar
> ELECTRON_RUN_AS_NODE=1 electron app.asar/index.js

Elevating
Executing: /Users/jviotti/Projects/playground/electron-elevate-asar/node_modules/electron-prebuilt/dist/Electron.app/Contents/MacOS/Electron /Users/jviotti/Projects/playground/electron-elevate-asar/app.asar/index.js
STDOUT: undefined
STDERR: undefined
/Users/jviotti/Projects/playground/electron-elevate-asar/app.asar/index.js:28
        throw error;
        ^

Error: Invalid package /usr/bin/sudo -n -E /Users/jviotti/Projects/playground/electron-elevate-asar/node_modules/electron-prebuilt/dist/Electron.app/Contents/MacOS/Electron /Users/jviotti/Projects/playground/electron-elevate-asar/app.asar
    at invalidArchiveError (ELECTRON_ASAR.js:137:13)
    at Object.module.(anonymous function) [as execFile] (ELECTRON_ASAR.js:192:16)
    at Object.exports.exec (child_process.js:111:18)
    at Attempt (/Users/jviotti/Projects/playground/electron-elevate-asar/node_modules/sudo-prompt/index.js:19:14)
    at Object.Exec (/Users/jviotti/Projects/playground/electron-elevate-asar/node_modules/sudo-prompt/index.js:105:3)
    at Immediate.<anonymous> (/Users/jviotti/Projects/playground/electron-elevate-asar/app.asar/index.js:21:23)
    at Immediate.immediate._onImmediate (timers.js:590:18)
    at tryOnImmediate (timers.js:543:15)
    at processImmediate [as _immediateCallback] (timers.js:523:5)

sudo-prompt executes the following command: sudo -n -E /Users/jviotti/Projects/playground/electron-elevate-asar/node_modules/electron-prebuilt/dist/Electron.app/Contents/MacOS/Electron /Users/jviotti/Projects/playground/electron-elevate-asar/app.asar/index.js, which as you can see above, seems to originate the issue, however copy-pasting this command into my terminal succeeds without any problem.

Basically, the elevation fails only then the command is executed from an electron process with ELECTRON_RUN_AS_NODE enabled.

The asar archive is of course valid, since the original script wouldn't run from it otherwise.

Some observations:

• The code runs fine when the script is not packaged inside an asar.
• The code works fine if we create another JavaScript file that executes sudo -n -E electron app.asar/index.js and we run it with NodeJS. Calling the same new file with ELECTRON_RUN_AS_NODE=1 electron exhibits the issue.

---

• Electron version: Ran in v0.37.6, v0.37.8 and v1.1.0.
• Operating system: OS X 10.11.4 and Ubuntu 14.04 x64.

[zcbenz]

Error: Invalid package /usr/bin/sudo -n -E /Users/jviotti/Projects/playground/electron-elevate-asar/node_modules/electron-prebuilt/dist/Electron.app/Contents/MacOS/Electron /Users/jviotti/Projects/playground/electron-elevate-asar/app.asar

From the error information the whole command /usr/bin/sudo -n -E ... is passed to child_process.execFile, instead of just the file. After looking into the implementation of child_process.exec, I found it just passes the whole command as file path to child_process.execFile.

Before this is fixed you have to modify the module to use execFile directly instead of exec to work around it.

[jviotti]

Hi @zcbenz ,

Thanks for your input. Using execFile indeed works correctly, and I can see how node passes the whole command to execFile in child_process.js.

By looking at Electron's child_process.execFile API override, I see that the whole command is detected to be an asar by the splitPath() function, causing getOrCreateArchive() to be called on the whole command.

What do you think could be a real solution to this issue? Happy to work on it myself if you want. I can think of the following options right now:

1. Override child_process.exec in lib/common/asar.js to split the command by white spaces, and call execFile by providing the first argument as the executable, although simply splitting the command might cause issues with paths including spaces (we could cover escaped white spaces though), and modifying the default child_process.exec behaviour might break existing code.
2. Add a check to splitPath() to perform the splitting as mentioned before, and potentially return another parameter in the resulting array containing the array of arguments, omitting the executable. This means that overrideAPI should be changed to handle the new array of arguments accordingly.
3. Add a check to splitPath to return isAsar == false if the command contains .asar, but the file path is part of a larger command, so the command is executable normally, without the asar layer.

The solution number 3 sounds like the most elegant one IMHO.

[jviotti]

Another potential workaround is to set process.noAsar = true right before the child_process.exec in sudo-prompt, and unset it afterwards.

[jviotti]

@zcbenz I'm working on the patch by following the third mentioned approach, but identifying the parts of the command is not something we could accurately do, since splitting by white spaces doesn't consider cases like paths containing spaces.

Some thoughts:

• We can split by whitespace and document a warning that commands containing white spaces lose asar support by default, which is of course not nice, and might break existing code.
• Split by absolute paths. We can split the command using a RegExp to only consider absolute paths as separate parts of the command. For example echo /path/to/my/asar with spaces.asar would be split into [ echo, /path/to/my/asar with spaces.asar ] but echo my/asar with spaces.asar would not be split at all. Finding a RegExp that covers all cases, Windows included could be a bit tricky to get right.
• We could be smarter in the way we detect something before or after the asar path. For example, if there is a space after .asar, which can very safely conclude there is something else afterwards. We could also check for an indicator of the beginning of an absolute or relative path before the .asar (for example: ./, /, <letter>:\, etc), and check if its index is 0 or -1. If that fails, we can assume there is something before.

What do you think?

I'm wondering if a better solution would be to accept a custom child_process.exec option to manually disable the asar support (e.g: electronNoAsar: true, or electronDisableOverride: true) and document it, since trying to make clever assumptions on the command mostly based on heuristics doesn't feel right, there are too many edge cases and ambiguities.

[zcbenz]

I think we can probably patch child_process.exec so it calls Node's original execFile instead of Electron's node.