Preserve user environment when executing command about sudo-prompt

[jorangreef]

Thanks @jviotti

To understand how sudo-prompt works see: #18 (comment)

The Attempt() call is just the tip of the iceberg. That's there to try and run the command if a sudo session already exists [...] But otherwise, the user command is run by a bash subshell which is called by the applet after acquiring administrator privileges.

[jviotti]

Hi @jorangreef ,

I inspected the applet zip after decoding it from index.js. After some more experimentation, my Electron application works fine, inheriting env vars correctly when the sudo timestamp is invalidated (the dialog is shown), however it fails afterwards, unless I run sudo -k.

Adding the -E option in Attempt() seems to solve the issue. As far as I can see Attempt() runs the command I passed to sudo-prompt to check if the process fails due to sudo requiring a password, or continues without problems.

Looks like the applet is handling the env var situation correctly already, so making sure Attempt() inherits the environment in case it succeeds is enough.

[jorangreef]

Thanks @jviotti, I saw that sudo's -E option can fail if denied by the user's sudoers policy:

sudo will return an error if the -E option is specified and the user does not have permission to preserve the environment.

I am not sure whether we will run into any -E errors in the wild, but if we do, then we will need to add support for explicit options.env in sudo-prompt rather than relying on the applet and sudo to preserve the user's environment.

For now though, we need to make a small change to the pull request (see there), and then it should be fine to land.

[jviotti]

@jorangreef I see. What about making this an opt-in option? Like preserveEnvironment: true?

[jorangreef]

I think it would be good to land the -E argument in Attempt() for now, but without making any promises in the documentation regarding preserving the environment.

Adding options tends to increase surface error for bugs, so if an option were to be added, I would rather add support for explicit options.env. It is lower level than preserveEnvironment (so preserving the environment could be built on top of that by the user) and also expected from the interface of child_process.exec().